From 3aff6f954613fff996407acf028617f6388e6161 Mon Sep 17 00:00:00 2001 From: Yuki Iwai Date: Wed, 16 Aug 2023 06:15:03 +0900 Subject: [PATCH] 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 * Initialize fullServiceDomain when adding certgenerator to the manager Signed-off-by: Yuki Iwai * Output logs every 15 seconds if the certs don't yet exist in the container Signed-off-by: Yuki Iwai --------- Signed-off-by: Yuki Iwai --- pkg/certgenerator/v1beta1/generator.go | 57 +++++++++++++++++-- pkg/certgenerator/v1beta1/generator_test.go | 62 +++++++++++++++++++++ 2 files changed, 113 insertions(+), 6 deletions(-) diff --git a/pkg/certgenerator/v1beta1/generator.go b/pkg/certgenerator/v1beta1/generator.go index f8c1d9520fa..d506f9a72ba 100644 --- 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 index 2e4f4a01105..5141720177d 100644 --- 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) + } + }) + } +}