address 2nd round comments

Author: Mengqi Yu

example/main.go

@@ -103,8 +103,7 @@ func main() {
 	as, err := webhook.NewServer("foo-admission-server", mgr, webhook.ServerOptions{
 		Port:    443,
 		CertDir: "/tmp/cert",
-		//Client:  mgr.GetClient(),
-		KVMap: map[string]interface{}{"foo": "bar"},
+		KVMap:   map[string]interface{}{"foo": "bar"},
 		BootstrapOptions: &webhook.BootstrapOptions{
 			Secret: &apitypes.NamespacedName{
 				Namespace: "default",

pkg/webhook/bootstrap.go

@@ -123,10 +123,10 @@ func (s *Server) setBootstrappingDefault() {
 	}
 }
 
-// bootstrap writes the configuration of admissionWebhookConfiguration in yaml format if dryrun is true.
+// installWebhookConfig writes the configuration of admissionWebhookConfiguration in yaml format if dryrun is true.
 // Otherwise, it creates the the admissionWebhookConfiguration objects and service if any.
 // It also provisions the certificate for the admission server.
-func (s *Server) bootstrap(dryrun bool) error {
+func (s *Server) installWebhookConfig() error {
 	// do defaulting if necessary
 	s.once.Do(s.setDefault)
 	if s.err != nil {
@@ -149,13 +149,13 @@ func (s *Server) bootstrap(dryrun bool) error {
 	_, err = s.certProvisioner.Provision(cert.Options{
 		ClientConfig: cc,
 		Objects:      s.webhookConfigurations,
-		Dryrun:       dryrun,
+		Dryrun:       s.Dryrun,
 	})
 	if err != nil {
 		return err
 	}
 
-	if dryrun {
+	if s.Dryrun {
 		// TODO: print here
 		// if dryrun, return the AdmissionWebhookConfiguration in yaml format.
 		return s.genYamlConfig(objects)

pkg/webhook/internal/cert/provisioner.go

@@ -45,7 +45,7 @@ type Options struct {
 	ClientConfig *admissionregistrationv1beta1.WebhookClientConfig
 	// Objects are the objects that will use the ClientConfig above.
 	Objects []runtime.Object
-	// Dryrun controls if the objects are sent to the API server or output in the io.Writer
+	// Dryrun controls if the objects are sent to the API server or write to io.Writer
 	Dryrun bool
 }
 

pkg/webhook/server.go

@@ -58,6 +58,11 @@ type ServerOptions struct {
 	// Client will be injected by the manager if not set.
 	Client client.Client
 
+	// Dryrun controls if the server will install the webhookConfiguration.
+	// If true, it will print the objects in yaml format.
+	// If false, it will install the objects in the cluster.
+	Dryrun bool
+
 	// BootstrapOptions contains the options for bootstrapping the admission server.
 	*BootstrapOptions
 }
@@ -177,10 +182,12 @@ func (s *Server) Register(webhooks ...Webhook) error {
 
 var _ manager.Runnable = &Server{}
 
-// Start runs the server.
+// Start runs the server if s.Dryrun is false.
+// Otherwise, it will print the objects in yaml format.
 func (s *Server) Start(stop <-chan struct{}) error {
-	err := s.bootstrap(false)
-	if err != nil {
+	err := s.installWebhookConfig()
+	// if encounter an error or it's in dryrun mode, return.
+	if err != nil || s.Dryrun {
 		return err
 	}
 
@@ -190,21 +197,17 @@ func (s *Server) Start(stop <-chan struct{}) error {
 	}
 	errCh := make(chan error)
 	serveFn := func() {
-		err := srv.ListenAndServeTLS(path.Join(s.CertDir, writer.ServerCertName), path.Join(s.CertDir, writer.ServerKeyName))
-		errCh <- err
+		errCh <- srv.ListenAndServeTLS(path.Join(s.CertDir, writer.ServerCertName), path.Join(s.CertDir, writer.ServerKeyName))
 	}
 
-	changed := true
+	go serveFn()
 	for {
 		// TODO(mengqiy): add jitter to the timer
 		// Could use https://godoc.org/k8s.io/apimachinery/pkg/util/wait#Jitter
 		timer := time.Tick(6 * 30 * 24 * time.Hour)
-		if changed {
-			go serveFn()
-		}
 		select {
 		case <-timer:
-			changed, err = s.RefreshCert()
+			changed, err := s.RefreshCert()
 			if err != nil {
 				log.Error(err, "encountering error when refreshing the certificate")
 				return err
@@ -218,6 +221,7 @@ func (s *Server) Start(stop <-chan struct{}) error {
 				log.Error(err, "encountering error when shutting down")
 				return err
 			}
+			go serveFn()
 		case <-stop:
 			return nil
 		case e := <-errCh:
@@ -226,11 +230,6 @@ func (s *Server) Start(stop <-chan struct{}) error {
 	}
 }
 
-// DryRun outputs k8s AdmissionWebhookConfiguration in yaml format instead of installing them to the APIServer.
-func (s *Server) DryRun() error {
-	return s.bootstrap(true)
-}
-
 // RefreshCert refreshes the certificate using Server's Provisioner if the certificate is expiring.
 func (s *Server) RefreshCert() (bool, error) {
 	cc, err := s.getClientConfig()