From da4b74da33f6edab9a6be36aefb2323765a18809 Mon Sep 17 00:00:00 2001 From: rchikatw Date: Fri, 12 Jan 2024 19:04:01 +0530 Subject: [PATCH] fixed re trigger job issue when public key deleted Signed-off-by: rchikatw --- controllers/storagecluster/provider_server.go | 2 + .../storagecluster_controller.go | 15 +- controllers/util/util.go | 4 + .../onboarding-secret-generator-role.yaml | 1 + onboarding/main.go | 135 ++++++++++-------- rbac/onboarding-secret-generator-role.yaml | 1 + tools/csv-merger/csv-merger.go | 6 +- 7 files changed, 92 insertions(+), 72 deletions(-) diff --git a/controllers/storagecluster/provider_server.go b/controllers/storagecluster/provider_server.go index f405a5d3b0..c7580615e2 100644 --- a/controllers/storagecluster/provider_server.go +++ b/controllers/storagecluster/provider_server.go @@ -454,6 +454,8 @@ func getOnboardingJobObject(instance *ocsv1.StorageCluster) *batchv1.Job { Namespace: instance.Namespace, }, Spec: batchv1.JobSpec{ + // Eligible to delete automatically when job finishes + TTLSecondsAfterFinished: util.ToPointer[int32](0), Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyOnFailure, diff --git a/controllers/storagecluster/storagecluster_controller.go b/controllers/storagecluster/storagecluster_controller.go index a850daac11..77e94341c1 100644 --- a/controllers/storagecluster/storagecluster_controller.go +++ b/controllers/storagecluster/storagecluster_controller.go @@ -166,13 +166,12 @@ func (r *StorageClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { }, } - onboardingSecretPredicates := builder.WithPredicates( - predicate.NewPredicateFuncs( - func(client client.Object) bool { - return client.GetName() == onboardingTicketPublicKeySecretName - }, - ), - ) + secretPredicate := predicate.Funcs{ + DeleteFunc: func(e event.DeleteEvent) bool { + // Evaluates to false if the object has been confirmed deleted. + return !e.DeleteStateUnknown + }, + } builder := ctrl.NewControllerManagedBy(mgr). For(&ocsv1.StorageCluster{}, builder.WithPredicates(scPredicate)). @@ -190,7 +189,7 @@ func (r *StorageClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { }, enqueueStorageClusterRequest, ). - Watches(&corev1.Secret{}, enqueueStorageClusterRequest, onboardingSecretPredicates). + Watches(&corev1.Secret{}, enqueueStorageClusterRequest, builder.WithPredicates(secretPredicate)). Watches(&ocsv1alpha1.StorageConsumer{}, enqueueStorageClusterRequest, builder.WithPredicates(ocsClientOperatorVersionPredicate)) if os.Getenv("SKIP_NOOBAA_CRD_WATCH") != "true" { builder.Owns(&nbv1.NooBaa{}) diff --git a/controllers/util/util.go b/controllers/util/util.go index f0d1ad6b93..8e0d9ea140 100644 --- a/controllers/util/util.go +++ b/controllers/util/util.go @@ -22,3 +22,7 @@ func DetectDuplicateInStringSlice(slice []string) bool { } return false } + +func ToPointer[T any](value T) *T { + return &value +} diff --git a/deploy/ocs-operator/manifests/onboarding-secret-generator-role.yaml b/deploy/ocs-operator/manifests/onboarding-secret-generator-role.yaml index cae160c88c..ff1e477f7d 100644 --- a/deploy/ocs-operator/manifests/onboarding-secret-generator-role.yaml +++ b/deploy/ocs-operator/manifests/onboarding-secret-generator-role.yaml @@ -11,3 +11,4 @@ rules: - get - list - create + - delete diff --git a/onboarding/main.go b/onboarding/main.go index 170feb4e46..2adb9f6883 100644 --- a/onboarding/main.go +++ b/onboarding/main.go @@ -18,80 +18,95 @@ import ( ) const ( - onboardingTicketPublicKeySecretName = "onboarding-ticket-key" //Name of existing public key which is used ocs-operator - onboardingPrivateKeySecretName = "onboarding-private-key" - serviceAccountName = "onboarding-secret-generator" + // Name of existing public key which is used ocs-operator + onboardingPublicKeySecretName = "onboarding-ticket-key" + onboardingPrivateKeySecretName = "onboarding-private-key" + serviceAccountName = "onboarding-secret-generator" ) func main() { clientset, err := newClient() if err != nil { - klog.Error(err, "failed to create controller-runtime client") + klog.Errorf("failed to create clientset: %v", err) return } operatorNamespace, err := util.GetOperatorNamespace() if err != nil { - klog.Error(err, "unable to get operator namespace") + klog.Errorf("unable to get operator namespace: %v", err) os.Exit(1) } - // 1. Check public key secret exist or not - _, err = clientset.CoreV1().Secrets(operatorNamespace).Get(context.TODO(), onboardingTicketPublicKeySecretName, metav1.GetOptions{}) - - if err != nil && kerrors.IsNotFound(err) { - // Generate RSA key. - var err error - privateKey, err := rsa.GenerateKey(rand.Reader, 4096) - if err != nil { - klog.Error(err, "unable to generate private") - os.Exit(1) - } - - publicKey := &privateKey.PublicKey - // Export the keys to pem string - privatePem := convertRsaPrivateKeyAsPemStr(privateKey) - publicPem, err := convertRsaPublicKeyAsPemStr(publicKey) - - if err != nil { - klog.Error(err, "failed to convert public key to pem str") - os.Exit(1) - } - - privateSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: onboardingPrivateKeySecretName, - Namespace: operatorNamespace, - Annotations: map[string]string{"kubernetes.io/service-account.name": serviceAccountName}, - }, - Type: "kubernetes.io/service-account-token", - StringData: map[string]string{ - "key": privatePem, - }, - } - - _, err = clientset.CoreV1().Secrets(operatorNamespace).Create(context.Background(), privateSecret, metav1.CreateOptions{}) - - if err != nil { - klog.Error(err, "Failed to create private secret.") - os.Exit(1) - } - publicSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: onboardingTicketPublicKeySecretName, - Namespace: operatorNamespace, - }, - StringData: map[string]string{ - "key": publicPem, - }, - } - - _, err = clientset.CoreV1().Secrets(operatorNamespace).Create(context.Background(), publicSecret, metav1.CreateOptions{}) - if err != nil { - klog.Error(err, "Failed to create public secret.") - os.Exit(1) - } + // Generate RSA key. + privateKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + klog.Errorf("unable to generate private: %v", err) + os.Exit(1) + } + + publicKey := &privateKey.PublicKey + // Export the keys to pem string + privatePem := convertRsaPrivateKeyAsPemStr(privateKey) + publicPem, err := convertRsaPublicKeyAsPemStr(publicKey) + + if err != nil { + klog.Errorf("failed to convert public key to pem str: %v", err) + os.Exit(1) + } + + // In situations where there is a risk of one secret being updated and potentially + // failing to update another, it is recommended not to rely solely on clientset update mechanisms. + // Instead, a safer approach is to delete both secrets and then recreate them simultaneously + // to ensure consistency and accuracy of all secrets. + + err = clientset.CoreV1().Secrets(operatorNamespace).Delete(context.Background(), onboardingPrivateKeySecretName, metav1.DeleteOptions{}) + + if err != nil && !kerrors.IsNotFound(err) { + klog.Errorf("failed to delete private secret: %v", err) + os.Exit(1) + } + + // Delete public key secret + err = clientset.CoreV1().Secrets(operatorNamespace).Delete(context.Background(), onboardingPublicKeySecretName, metav1.DeleteOptions{}) + if err != nil && !kerrors.IsNotFound(err) { + klog.Errorf("failed to delete public secret: %v", err) + os.Exit(1) + } + + privateSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: onboardingPrivateKeySecretName, + Namespace: operatorNamespace, + Annotations: map[string]string{"kubernetes.io/service-account.name": serviceAccountName}, + }, + Type: "kubernetes.io/service-account-token", + StringData: map[string]string{ + "key": privatePem, + }, + } + + _, err = clientset.CoreV1().Secrets(operatorNamespace).Create(context.Background(), privateSecret, metav1.CreateOptions{}) + + if err != nil { + klog.Errorf("failed to create private secret: %v", err) + os.Exit(1) + } + + publicSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: onboardingPublicKeySecretName, + Namespace: operatorNamespace, + }, + StringData: map[string]string{ + "key": publicPem, + }, + } + + _, err = clientset.CoreV1().Secrets(operatorNamespace).Create(context.Background(), publicSecret, metav1.CreateOptions{}) + if err != nil { + klog.Errorf("failed to create public secret: %v", err) + os.Exit(1) } } diff --git a/rbac/onboarding-secret-generator-role.yaml b/rbac/onboarding-secret-generator-role.yaml index cae160c88c..ff1e477f7d 100644 --- a/rbac/onboarding-secret-generator-role.yaml +++ b/rbac/onboarding-secret-generator-role.yaml @@ -11,3 +11,4 @@ rules: - get - list - create + - delete diff --git a/tools/csv-merger/csv-merger.go b/tools/csv-merger/csv-merger.go index 21c715c4bd..dfcaf41448 100644 --- a/tools/csv-merger/csv-merger.go +++ b/tools/csv-merger/csv-merger.go @@ -925,10 +925,8 @@ func copyManifests() { } func getUXBackendServerDeployment() appsv1.DeploymentSpec { - replica := int32(1) - ptrToTrue := true deployment := appsv1.DeploymentSpec{ - Replicas: &replica, + Replicas: util.ToPointer[int32](1), Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "app.kubernetes.io/component": "ux-backend-server", @@ -1019,7 +1017,7 @@ func getUXBackendServerDeployment() appsv1.DeploymentSpec { VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ SecretName: "onboarding-private-key", - Optional: &ptrToTrue, + Optional: util.ToPointer[bool](true), }, }, },