Bug: Wait for the certs to be mounted inside the container (#2198)

* Wait for the certs to be mounted inside the container Signed-off-by: Yuki Iwai <[email protected]> * Initialize fullServiceDomain when adding certgenerator to the manager Signed-off-by: Yuki Iwai <[email protected]> * Output logs every 15 seconds if the certs don't yet exist in the container Signed-off-by: Yuki Iwai <[email protected]> --------- Signed-off-by: Yuki Iwai <[email protected]>
kubeflow · Aug 15, 2023 · 1b68744 · 1b68744
1 parent 2ae3eb5
commit 1b68744
Show file tree

Hide file tree

Showing 2 changed files with 113 additions and 6 deletions.
diff --git a/pkg/certgenerator/v1beta1/generator.go b/pkg/certgenerator/v1beta1/generator.go
@@ -26,12 +26,15 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
+	"os"
+	"path/filepath"
 	"strings"
 	"time"
 
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/klog"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -53,11 +56,11 @@ type CertGenerator struct {
 	namespace          string
 	webhookServiceName string
 	webhookSecretName  string
+	fullServiceDomain  string
 	kubeClient         client.Client
 	certsReady         chan struct{}
 
-	certs             *certificates
-	fullServiceDomain string
+	certs *certificates
 }
 
 var _ manager.Runnable = &CertGenerator{}
@@ -67,11 +70,50 @@ func (c *CertGenerator) Start(ctx context.Context) error {
 	if err := c.generate(ctx); err != nil {
 		return err
 	}
+	klog.Info("Waiting for certs to get ready.")
+	if err := wait.ExponentialBackoffWithContext(ctx, wait.Backoff{
+		Duration: time.Second,
+		Factor:   2,
+		Jitter:   1,
+		Steps:    10,
+		Cap:      time.Minute * 5,
+	}, ensureCertMounted(time.Now())); err != nil {
+		return err
+	}
 	// Sending an empty data to a certsReady means it starts to register controllers to the manager.
 	c.certsReady <- struct{}{}
 	return nil
 }
 
+// ensureCertMounted ensures that the generated certs are mounted inside the container.
+func ensureCertMounted(start time.Time) func(context.Context) (bool, error) {
+	return func(ctx context.Context) (bool, error) {
+		now := time.Now()
+		outputLog := false
+		if now.Sub(start) >= 15*time.Second {
+			start = now
+			outputLog = true
+		}
+
+		certFile := filepath.Join(consts.CertDir, serverCertName)
+		if _, err := os.Stat(certFile); err != nil {
+			if outputLog {
+				klog.Infof("Public key file %q doesn't exist in the container yet", certFile)
+			}
+			return false, nil
+		}
+		keyFile := filepath.Join(consts.CertDir, serverKeyName)
+		if _, err := os.Stat(keyFile); err != nil {
+			if outputLog {
+				klog.Infof("Private key file %q doesn't exist in the container yet", keyFile)
+			}
+			return false, nil
+		}
+		klog.Info("Succeeded to be mounted certs inside the container.")
+		return true, nil
+	}
+}
+
 func (c *CertGenerator) NeedLeaderElection() bool {
 	return false
 }
@@ -82,8 +124,13 @@ func AddToManager(mgr manager.Manager, config configv1beta1.CertGeneratorConfig,
 		namespace:          consts.DefaultKatibNamespace,
 		webhookServiceName: config.WebhookServiceName,
 		webhookSecretName:  config.WebhookSecretName,
-		kubeClient:         mgr.GetClient(),
-		certsReady:         certsReady,
+		fullServiceDomain: strings.Join([]string{
+			config.WebhookServiceName,
+			consts.DefaultKatibNamespace,
+			"svc",
+		}, "."),
+		kubeClient: mgr.GetClient(),
+		certsReady: certsReady,
 	})
 }
 
@@ -99,8 +146,6 @@ func (c *CertGenerator) generate(ctx context.Context) error {
 		return fmt.Errorf("%w: %v", errCertCheckFail, err)
 	}
 	if !certExist {
-		c.fullServiceDomain = strings.Join([]string{c.webhookServiceName, c.namespace, "svc"}, ".")
-
 		if err = c.createCert(); err != nil {
 			return fmt.Errorf("%w: %v", errCreateCertFail, err)
 		}

diff --git a/pkg/certgenerator/v1beta1/generator_test.go b/pkg/certgenerator/v1beta1/generator_test.go
@@ -18,8 +18,11 @@ package certgenerator
 
 import (
 	"context"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -31,6 +34,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	configv1beta1 "github.com/kubeflow/katib/pkg/apis/config/v1beta1"
+	"github.com/kubeflow/katib/pkg/controller.v1beta1/consts"
 )
 
 func TestGenerate(t *testing.T) {
@@ -210,3 +214,61 @@ func buildFakeClient(kubeResources []client.Object) client.Client {
 	}
 	return fakeClientBuilder.Build()
 }
+
+func TestEnsureCertMounted(t *testing.T) {
+	tests := map[string]struct {
+		keyExist  bool
+		certExist bool
+		wantExist bool
+	}{
+		"key and cert exist": {
+			keyExist:  true,
+			certExist: true,
+			wantExist: true,
+		},
+		"key doesn't exist": {
+			keyExist:  false,
+			certExist: true,
+			wantExist: false,
+		},
+		"cert doesn't exist": {
+			keyExist:  true,
+			certExist: false,
+			wantExist: false,
+		},
+		"all files doesn't exist": {
+			keyExist:  false,
+			certExist: false,
+			wantExist: false,
+		},
+	}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			if tc.keyExist || tc.certExist {
+				if err := os.MkdirAll(consts.CertDir, 0760); err != nil {
+					t.Fatalf("Failed to set up directory: %v", err)
+				}
+				defer func() {
+					if err := os.RemoveAll(consts.CertDir); err != nil {
+						t.Fatalf("Failed to clean up directory: %v", err)
+					}
+				}()
+			}
+			if tc.keyExist {
+				if _, err := os.Create(filepath.Join(consts.CertDir, serverKeyName)); err != nil {
+					t.Fatalf("Failed to create tls.key: %v", err)
+				}
+			}
+			if tc.certExist {
+				if _, err := os.Create(filepath.Join(consts.CertDir, serverCertName)); err != nil {
+					t.Fatalf("Failed to create tls.crt: %v", err)
+				}
+			}
+			ensureFunc := ensureCertMounted(time.Now())
+			got, _ := ensureFunc(context.Background())
+			if tc.wantExist != got {
+				t.Errorf("Unexpected value from ensureCertMounted: \n(want: %v, got: %v)\n", tc.wantExist, got)
+			}
+		})
+	}
+}