From c9a266820ba1612831420ce0a71b1c79d096575a Mon Sep 17 00:00:00 2001 From: "michal.gubricky" Date: Fri, 16 Feb 2024 15:44:33 +0100 Subject: [PATCH 1/8] Add default values for the 'cloudName' and 'identityRef' fields Signed-off-by: michal.gubricky --- README.md | 3 + Tiltfile | 2 +- api/v1alpha1/conditions_const.go | 3 + .../openstackclusterstackrelease_types.go | 5 +- ...-k8s.io_openstackclusterstackreleases.yaml | 11 +-- ...openstackclusterstackreleasetemplates.yaml | 11 +-- config/cspo/cspotemplate.yaml | 11 +-- config/cspo/secret.yaml | 7 +- examples/cspotemplate.yaml | 3 +- ...openstackclusterstackrelease_controller.go | 48 ++++++++++- ...tackclusterstackrelease_controller_test.go | 80 ++++++++++++++++--- .../integration/github/integration_test.go | 19 ++++- .../integration/openstack/controller_test.go | 1 - 13 files changed, 162 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index e42343ae..ac6d9d5c 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,9 @@ clusterctl init --infrastructure openstack To enable communication between the CSPO and the Cluster API Provider for OpenStack (CAPO) with the OpenStack API, it is necessary to generate a secret containing the access data (clouds.yaml). Ensure that this secret is located in the identical namespace as the other Custom Resources. +> [!NOTE] +> The default value of `cloudName` is configured as `openstack`. This setting can be overridden by including the `cloudName` key in the secret. Also, be aware that the name of the secret is expected to be `openstack` unless it is not set differently in OpenStackClusterStackReleaseTemplate in `identityRef.name` field. + ```bash kubectl create secret generic --from-file=clouds.yaml=path/to/clouds.yaml diff --git a/Tiltfile b/Tiltfile index 69bd2c00..abf8959b 100644 --- a/Tiltfile +++ b/Tiltfile @@ -268,7 +268,7 @@ def deploy_cspo(): def create_secret(): cmd = "cat .secret.yaml | {} | kubectl apply -f -".format(envsubst_cmd) - local_resource('supersecret', cmd, labels=["clouds-yaml-secret"]) + local_resource('supersecret', cmd, labels=["clouds-yaml-secret"]) def cspo_template(): cmd = "cat .cspotemplate.yaml | {}".format(envsubst_cmd) diff --git a/api/v1alpha1/conditions_const.go b/api/v1alpha1/conditions_const.go index d2d0a5bb..0e2b9cf8 100644 --- a/api/v1alpha1/conditions_const.go +++ b/api/v1alpha1/conditions_const.go @@ -48,6 +48,9 @@ const ( ) const ( + // CloudNameAvailableCondition is used when cloud name is available. + CloudNameAvailableCondition = "CloudNameAvailable" + // CloudAvailableCondition is used when cloud is available. CloudAvailableCondition = "CloudAvailable" diff --git a/api/v1alpha1/openstackclusterstackrelease_types.go b/api/v1alpha1/openstackclusterstackrelease_types.go index 79416ce6..e9e87cda 100644 --- a/api/v1alpha1/openstackclusterstackrelease_types.go +++ b/api/v1alpha1/openstackclusterstackrelease_types.go @@ -27,10 +27,9 @@ import ( // OpenStackClusterStackReleaseSpec defines the desired state of OpenStackClusterStackRelease. type OpenStackClusterStackReleaseSpec struct { - // CloudName is the name of the cloud to use from the cloud's secret. - // +kubebuilder:validation:MinLength=1 - CloudName string `json:"cloudName"` // IdentityRef is a reference to a identity to be used when reconciling this cluster + // +optional + // +kubebuilder:default:={kind: "Secret", name: "openstack"} IdentityRef *apiv1alpha7.OpenStackIdentityReference `json:"identityRef"` } diff --git a/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleases.yaml b/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleases.yaml index b1a642b2..deedd063 100644 --- a/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleases.yaml +++ b/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleases.yaml @@ -52,12 +52,10 @@ spec: description: OpenStackClusterStackReleaseSpec defines the desired state of OpenStackClusterStackRelease. properties: - cloudName: - description: CloudName is the name of the cloud to use from the cloud's - secret. - minLength: 1 - type: string identityRef: + default: + kind: Secret + name: openstack description: IdentityRef is a reference to a identity to be used when reconciling this cluster properties: @@ -75,9 +73,6 @@ spec: - kind - name type: object - required: - - cloudName - - identityRef type: object status: description: OpenStackClusterStackReleaseStatus defines the observed state diff --git a/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleasetemplates.yaml b/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleasetemplates.yaml index 54f559e8..f99d407d 100644 --- a/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleasetemplates.yaml +++ b/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstackclusterstackreleasetemplates.yaml @@ -47,12 +47,10 @@ spec: description: OpenStackClusterStackReleaseSpec defines the desired state of OpenStackClusterStackRelease. properties: - cloudName: - description: CloudName is the name of the cloud to use from - the cloud's secret. - minLength: 1 - type: string identityRef: + default: + kind: Secret + name: openstack description: IdentityRef is a reference to a identity to be used when reconciling this cluster properties: @@ -72,9 +70,6 @@ spec: - kind - name type: object - required: - - cloudName - - identityRef type: object required: - spec diff --git a/config/cspo/cspotemplate.yaml b/config/cspo/cspotemplate.yaml index 4f1d1522..c171706c 100644 --- a/config/cspo/cspotemplate.yaml +++ b/config/cspo/cspotemplate.yaml @@ -5,8 +5,9 @@ metadata: namespace: cluster spec: template: - spec: - cloudName: "${CLOUD_NAME}" - identityRef: - kind: Secret - name: "${SECRET_NAME}" + spec: {} + # Field identityRef is optional and its default values ​​are as follows: + # identityRef.kind: "Secret", identityRef.name: "openstack" + # identityRef: + # kind: Secret + # name: "" diff --git a/config/cspo/secret.yaml b/config/cspo/secret.yaml index cd6f0e68..10412617 100644 --- a/config/cspo/secret.yaml +++ b/config/cspo/secret.yaml @@ -1,9 +1,14 @@ apiVersion: v1 data: + # The default value of `cloudName` is configured as `openstack`. + # This can be overridden by including the `cloudName` key in this secret. + # cloudName: "openstack" clouds.yaml: ${ENCODED_CLOUDS_YAML} kind: Secret metadata: labels: clusterctl.cluster.x-k8s.io/move: "true" - name: "${SECRET_NAME}" + # Note: Value of the field `name` must be the same as the value of field + # `identityRef.name` in OpenStackClusterStackReleaseTemplate object. + name: "openstack" namespace: cluster diff --git a/examples/cspotemplate.yaml b/examples/cspotemplate.yaml index b8550695..376277ec 100644 --- a/examples/cspotemplate.yaml +++ b/examples/cspotemplate.yaml @@ -5,7 +5,8 @@ metadata: spec: template: spec: - cloudName: + # Field identityRef is optional and its default values ​​are as follows: + # identityRef.kind: "Secret", identityRef.name: "openstack" identityRef: kind: Secret name: diff --git a/internal/controller/openstackclusterstackrelease_controller.go b/internal/controller/openstackclusterstackrelease_controller.go index 9b9b76bd..2f8f4520 100644 --- a/internal/controller/openstackclusterstackrelease_controller.go +++ b/internal/controller/openstackclusterstackrelease_controller.go @@ -29,6 +29,7 @@ import ( githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/release" apiv1alpha1 "github.com/sovereignCloudStack/cluster-stack-provider-openstack/api/v1alpha1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -60,6 +61,7 @@ type NodeImages struct { } const ( + cloudNameSecretKey = "cloudName" metadataFileName = "metadata.yaml" nodeImagesFileName = "node-images.yaml" waitForOpenStackNodeImageReleasesBecomeReady = 30 * time.Second @@ -174,8 +176,22 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context, } osnirName := fmt.Sprintf("%s-%s-%s", nameWithoutVersion, openStackNodeImage.CreateOpts.Name, nodeImageVersion) + cloudName, err := r.getCloudNameFromSecret(ctx, openstackclusterstackrelease.Namespace, openstackclusterstackrelease.Spec.IdentityRef.Name) + if err != nil { + if apierrors.IsNotFound(err) { + conditions.MarkFalse(openstackclusterstackrelease, + apiv1alpha1.CloudNameAvailableCondition, + apiv1alpha1.SecretNotFoundReason, + clusterv1beta1.ConditionSeverityError, + err.Error(), + ) + record.Warnf(openstackclusterstackrelease, "SecretNotFound", err.Error()) + logger.Error(err, "failed to get secret") + return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil + } + } - if err := r.createOrUpdateOpenStackNodeImageRelease(ctx, openstackclusterstackrelease, osnirName, openStackNodeImage, ownerRef); err != nil { + if err := r.createOrUpdateOpenStackNodeImageRelease(ctx, openstackclusterstackrelease, osnirName, cloudName, openStackNodeImage, ownerRef); err != nil { return ctrl.Result{}, fmt.Errorf("failed to create or update OpenStackNodeImageRelease %s/%s: %w", openstackclusterstackrelease.Namespace, osnirName, err) } } @@ -218,7 +234,7 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context, return ctrl.Result{}, nil } -func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImageRelease(ctx context.Context, openstackclusterstackrelease *apiv1alpha1.OpenStackClusterStackRelease, osnirName string, openStackNodeImage *apiv1alpha1.OpenStackNodeImage, ownerRef *metav1.OwnerReference) error { +func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImageRelease(ctx context.Context, openstackclusterstackrelease *apiv1alpha1.OpenStackClusterStackRelease, osnirName, cloudName string, openStackNodeImage *apiv1alpha1.OpenStackNodeImage, ownerRef *metav1.OwnerReference) error { openStackNodeImageRelease := &apiv1alpha1.OpenStackNodeImageRelease{} err := r.Get(ctx, types.NamespacedName{Name: osnirName, Namespace: openstackclusterstackrelease.Namespace}, openStackNodeImageRelease) @@ -250,7 +266,7 @@ func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImag } openStackNodeImageRelease.SetOwnerReferences([]metav1.OwnerReference{*ownerRef}) openStackNodeImageRelease.Spec.Image = openStackNodeImage - openStackNodeImageRelease.Spec.CloudName = openstackclusterstackrelease.Spec.CloudName + openStackNodeImageRelease.Spec.CloudName = cloudName openStackNodeImageRelease.Spec.IdentityRef = openstackclusterstackrelease.Spec.IdentityRef if err := r.Create(ctx, openStackNodeImageRelease); err != nil { @@ -350,6 +366,32 @@ func cutOpenStackClusterStackReleaseVersionFromReleaseTag(releaseTag string) (st return fmt.Sprintf("%s-%s-%s-%s", v[0], v[1], v[2], v[3]), nil } +func (r *OpenStackClusterStackReleaseReconciler) getCloudNameFromSecret(ctx context.Context, secretNamespace, secretName string) (string, error) { + var cloudName string + emptyCloudName := "" + defaultCloudName := "openstack" + + secret := &corev1.Secret{} + err := r.Get(ctx, types.NamespacedName{ + Namespace: secretNamespace, + Name: secretName, + }, secret) + if err != nil { + return emptyCloudName, fmt.Errorf("failed to get secret %s in namespace %s: %w", secretName, secretNamespace, err) + } + + content, ok := secret.Data[cloudNameSecretKey] + if !ok { + return defaultCloudName, nil + } + + if err := yaml.Unmarshal(content, &cloudName); err != nil { + return emptyCloudName, fmt.Errorf("failed to unmarshal cloudName stored in secret %s: %w", secretName, err) + } + + return cloudName, nil +} + // SetupWithManager sets up the controller with the Manager. func (r *OpenStackClusterStackReleaseReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/internal/controller/openstackclusterstackrelease_controller_test.go b/internal/controller/openstackclusterstackrelease_controller_test.go index 7d9311f7..a58a5c35 100644 --- a/internal/controller/openstackclusterstackrelease_controller_test.go +++ b/internal/controller/openstackclusterstackrelease_controller_test.go @@ -74,7 +74,6 @@ func TestGenerateOwnerReference(t *testing.T) { UID: "fb686e33-01a6-42c9-a210-2c26ec8cb331", }, Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{ - CloudName: "openstack", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", @@ -103,7 +102,6 @@ func TestMatchOwnerReference(t *testing.T) { Name: "openstack-ferrol-1-27-v1", }, Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{ - CloudName: "openstack", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret1", @@ -122,7 +120,6 @@ func TestMatchOwnerReference(t *testing.T) { Name: "openstack-ferrol-1-27-v2", }, Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{ - CloudName: "openstack", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret2", @@ -245,7 +242,6 @@ func TestGetOwnedOpenStackNodeImageReleases(t *testing.T) { Namespace: "test-namespace", }, Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{ - CloudName: "test-cloudname", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", @@ -290,6 +286,8 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) { scheme := runtime.NewScheme() err := apiv1alpha1.AddToScheme(scheme) assert.NoError(t, err) + err = corev1.AddToScheme(scheme) + assert.NoError(t, err) client := fake.NewClientBuilder().WithScheme(scheme).Build() openstackclusterstackrelease := &apiv1alpha1.OpenStackClusterStackRelease{ @@ -302,7 +300,6 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) { Namespace: "test-namespace", }, Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{ - CloudName: "test-cloudname", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", @@ -328,11 +325,26 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) { UID: openstackclusterstackrelease.UID, } + secretName := "supersecret" + secretNamespace := "test-namespace" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: secretNamespace, + }, + Type: corev1.SecretTypeOpaque, + } + err = client.Create(context.TODO(), secret) + assert.NoError(t, err) + r := &OpenStackClusterStackReleaseReconciler{ Client: client, } - err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-osnir", openStackNodeImage, ownerRef) + cloudName, err := r.getCloudNameFromSecret(context.TODO(), secretNamespace, secretName) + assert.NoError(t, err) + + err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-osnir", cloudName, openStackNodeImage, ownerRef) assert.NoError(t, err) osnir := &apiv1alpha1.OpenStackNodeImageRelease{} @@ -346,7 +358,7 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) { APIVersion: apiv1alpha1.GroupVersion.String(), }, Spec: apiv1alpha1.OpenStackNodeImageReleaseSpec{ - CloudName: "test-cloudname", + CloudName: "openstack", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", @@ -388,7 +400,6 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) { Namespace: "test-namespace", }, Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{ - CloudName: "test-cloudname", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", @@ -458,7 +469,7 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) { assert.NoError(t, err) assert.Equal(t, ownerRef.UID, osnir.OwnerReferences[0].UID) - err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-update-osnir", openStackNodeImage, newOwnerRef) + err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-update-osnir", "test-cloud-name", openStackNodeImage, newOwnerRef) assert.NoError(t, err) err = client.Get(context.TODO(), types.NamespacedName{Name: "test-update-osnir", Namespace: "test-namespace"}, osnir) @@ -473,6 +484,56 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) { assert.NoError(t, err) } +func TestGetCloudNameFromSecret(t *testing.T) { + client := fake.NewClientBuilder().Build() + + secretName := "supersecret" + secretNamespace := "test-namespace" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: secretNamespace, + }, + Type: corev1.SecretTypeOpaque, + } + err := client.Create(context.TODO(), secret) + assert.NoError(t, err) + + r := &OpenStackClusterStackReleaseReconciler{ + Client: client, + } + + cloudName, err := r.getCloudNameFromSecret(context.TODO(), secretNamespace, secretName) + expectedCloudName := "openstack" + + assert.NoError(t, err) + assert.Equal(t, expectedCloudName, cloudName) + + err = client.Delete(context.TODO(), secret) + assert.NoError(t, err) +} + +func TestGetCloudNameFromSecretNotFound(t *testing.T) { + client := fake.NewClientBuilder().Build() + + r := &OpenStackClusterStackReleaseReconciler{ + Client: client, + } + + secretName := "nonexistent-secret" + secretNamespace := "nonexistent-namespace" + expectedError := "secrets \"nonexistent-secret\" not found" + + cloudName, err := r.getCloudNameFromSecret(context.TODO(), secretNamespace, secretName) + + expectedErrorMessage := fmt.Sprintf("failed to get secret %s in namespace %s: %v", secretName, secretNamespace, expectedError) + + assert.Error(t, err) + assert.True(t, apierrors.IsNotFound(err)) + assert.Equal(t, "", cloudName) + assert.EqualError(t, err, expectedErrorMessage) +} + var _ = Describe("OpenStackClusterStackRelease controller", func() { Context("OpenStackClusterStackRelease controller test", func() { const openstackclusterstackreleasename = "test-ocsr" @@ -510,7 +571,6 @@ var _ = Describe("OpenStackClusterStackRelease controller", func() { Namespace: namespace.Name, }, Spec: apiv1alpha1.OpenStackClusterStackReleaseSpec{ - CloudName: "openstack", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", diff --git a/internal/test/integration/github/integration_test.go b/internal/test/integration/github/integration_test.go index 507e68db..6882c160 100644 --- a/internal/test/integration/github/integration_test.go +++ b/internal/test/integration/github/integration_test.go @@ -17,6 +17,9 @@ limitations under the License. package github import ( + "encoding/base64" + "os" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/test/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -43,6 +46,21 @@ var _ = Describe("OpenStackClusterStackReleaseReconciler", func() { openstackClusterStackReleaseKey = types.NamespacedName{Name: "openstack-scs-1-27-v2", Namespace: testNs.Name} + cloudsYAMLBase64 := os.Getenv("ENCODED_CLOUDS_YAML") + cloudsYAMLData, err := base64.StdEncoding.DecodeString(cloudsYAMLBase64) + Expect(err).NotTo(HaveOccurred()) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "supersecret", + Namespace: testNs.Name, + }, + Data: map[string][]byte{ + "clouds.yaml": cloudsYAMLData, + }, + } + Expect(testEnv.Create(ctx, secret)).To(Succeed()) + openStackClusterStackRelease = &cspov1alpha1.OpenStackClusterStackRelease{ TypeMeta: metav1.TypeMeta{ Kind: "OpenStackClusterStackRelease", @@ -53,7 +71,6 @@ var _ = Describe("OpenStackClusterStackReleaseReconciler", func() { Namespace: testNs.Name, }, Spec: cspov1alpha1.OpenStackClusterStackReleaseSpec{ - CloudName: "capi-openstack-scs-1-27-v2", IdentityRef: &apiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", diff --git a/internal/test/integration/openstack/controller_test.go b/internal/test/integration/openstack/controller_test.go index b1ae0764..b884e918 100644 --- a/internal/test/integration/openstack/controller_test.go +++ b/internal/test/integration/openstack/controller_test.go @@ -69,7 +69,6 @@ var _ = Describe("OpenStackNodeImageReleaseReconciler", func() { Namespace: testNs.Name, }, Spec: cspov1alpha1.OpenStackClusterStackReleaseSpec{ - CloudName: "openstack", IdentityRef: &apiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", From e18affdc1345e6a2afc043cd429f4555cce02d39 Mon Sep 17 00:00:00 2001 From: "michal.gubricky" Date: Tue, 20 Feb 2024 14:41:15 +0100 Subject: [PATCH 2/8] Catch unknown error from the getCloudNameFromSecret function Signed-off-by: michal.gubricky --- api/v1alpha1/conditions_const.go | 3 +++ .../openstackclusterstackrelease_controller.go | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/api/v1alpha1/conditions_const.go b/api/v1alpha1/conditions_const.go index 0e2b9cf8..321a0c8e 100644 --- a/api/v1alpha1/conditions_const.go +++ b/api/v1alpha1/conditions_const.go @@ -59,6 +59,9 @@ const ( // SecretNotFoundReason is used when the secret specified by the user is not found. SecretNotFoundReason = "SecretNotFound" + + // IssueWithSecretReason is used when getting the key-value pair from the secret failed. + IssueWithSecretReason = "IssueWithSecret" ) const ( diff --git a/internal/controller/openstackclusterstackrelease_controller.go b/internal/controller/openstackclusterstackrelease_controller.go index 2f8f4520..32eb9cf0 100644 --- a/internal/controller/openstackclusterstackrelease_controller.go +++ b/internal/controller/openstackclusterstackrelease_controller.go @@ -189,8 +189,18 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context, logger.Error(err, "failed to get secret") return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil } + conditions.MarkFalse(openstackclusterstackrelease, + apiv1alpha1.CloudNameAvailableCondition, + apiv1alpha1.IssueWithSecretReason, + clusterv1beta1.ConditionSeverityError, + err.Error(), + ) + record.Warnf(openstackclusterstackrelease, "IssueWithSecret", err.Error()) + return ctrl.Result{}, fmt.Errorf("failed to get cloud name from secret: %w", err) } + conditions.MarkTrue(openstackclusterstackrelease, apiv1alpha1.CloudNameAvailableCondition) + if err := r.createOrUpdateOpenStackNodeImageRelease(ctx, openstackclusterstackrelease, osnirName, cloudName, openStackNodeImage, ownerRef); err != nil { return ctrl.Result{}, fmt.Errorf("failed to create or update OpenStackNodeImageRelease %s/%s: %w", openstackclusterstackrelease.Namespace, osnirName, err) } From 26ae5702263dff4ffa8393ec27ab0e71bba5550b Mon Sep 17 00:00:00 2001 From: "michal.gubricky" Date: Tue, 20 Feb 2024 15:19:39 +0100 Subject: [PATCH 3/8] Change the severity of unknown error to warning Signed-off-by: michal.gubricky --- internal/controller/openstackclusterstackrelease_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/openstackclusterstackrelease_controller.go b/internal/controller/openstackclusterstackrelease_controller.go index 32eb9cf0..8b735c59 100644 --- a/internal/controller/openstackclusterstackrelease_controller.go +++ b/internal/controller/openstackclusterstackrelease_controller.go @@ -192,7 +192,7 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context, conditions.MarkFalse(openstackclusterstackrelease, apiv1alpha1.CloudNameAvailableCondition, apiv1alpha1.IssueWithSecretReason, - clusterv1beta1.ConditionSeverityError, + clusterv1beta1.ConditionSeverityWarning, err.Error(), ) record.Warnf(openstackclusterstackrelease, "IssueWithSecret", err.Error()) From df0cacb2d12e7bc5f6cfd0099fcba5d3a267be59 Mon Sep 17 00:00:00 2001 From: "michal.gubricky" Date: Fri, 23 Feb 2024 14:33:11 +0100 Subject: [PATCH 4/8] Fix yaml lint warnings Signed-off-by: michal.gubricky --- config/cspo/cspotemplate.yaml | 8 ++++---- examples/cspotemplate.yaml | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/cspo/cspotemplate.yaml b/config/cspo/cspotemplate.yaml index c171706c..00a2d98b 100644 --- a/config/cspo/cspotemplate.yaml +++ b/config/cspo/cspotemplate.yaml @@ -6,8 +6,8 @@ metadata: spec: template: spec: {} - # Field identityRef is optional and its default values ​​are as follows: + # Field identityRef is optional and its default values are as follows: # identityRef.kind: "Secret", identityRef.name: "openstack" - # identityRef: - # kind: Secret - # name: "" + # identityRef: + # kind: Secret + # name: "" diff --git a/examples/cspotemplate.yaml b/examples/cspotemplate.yaml index 376277ec..cd163212 100644 --- a/examples/cspotemplate.yaml +++ b/examples/cspotemplate.yaml @@ -5,8 +5,8 @@ metadata: spec: template: spec: - # Field identityRef is optional and its default values ​​are as follows: - # identityRef.kind: "Secret", identityRef.name: "openstack" + # Field identityRef is optional and its default values ​​are as follows: + # identityRef.kind: "Secret", identityRef.name: "openstack" identityRef: kind: Secret name: From 74a6a87ccf42e3e22dd3234e691d76c38f6884f4 Mon Sep 17 00:00:00 2001 From: Michal Gubricky Date: Fri, 23 Feb 2024 15:13:46 +0100 Subject: [PATCH 5/8] Update config/cspo/secret.yaml Co-authored-by: Matej Feder Signed-off-by: Michal Gubricky --- config/cspo/secret.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/cspo/secret.yaml b/config/cspo/secret.yaml index 10412617..d6b20c71 100644 --- a/config/cspo/secret.yaml +++ b/config/cspo/secret.yaml @@ -8,7 +8,7 @@ kind: Secret metadata: labels: clusterctl.cluster.x-k8s.io/move: "true" - # Note: Value of the field `name` must be the same as the value of field + # Note: `metadata.name` must be the same as the value of the field # `identityRef.name` in OpenStackClusterStackReleaseTemplate object. name: "openstack" namespace: cluster From 8921785214651fcebc05dc57b17bee7f781e2378 Mon Sep 17 00:00:00 2001 From: "michal.gubricky" Date: Fri, 23 Feb 2024 15:51:57 +0100 Subject: [PATCH 6/8] Replace with openstack in docs Signed-off-by: michal.gubricky --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ac6d9d5c..e85d2643 100644 --- a/README.md +++ b/README.md @@ -50,11 +50,11 @@ Ensure that this secret is located in the identical namespace as the other Custo > The default value of `cloudName` is configured as `openstack`. This setting can be overridden by including the `cloudName` key in the secret. Also, be aware that the name of the secret is expected to be `openstack` unless it is not set differently in OpenStackClusterStackReleaseTemplate in `identityRef.name` field. ```bash -kubectl create secret generic --from-file=clouds.yaml=path/to/clouds.yaml +kubectl create secret generic openstack --from-file=clouds.yaml=path/to/clouds.yaml # Patch the created secrets so they are automatically moved to the target cluster later. -kubectl patch secret -p '{"metadata":{"labels":{"clusterctl.cluster.x-k8s.io/move":""}}}' +kubectl patch secret openstack -p '{"metadata":{"labels":{"clusterctl.cluster.x-k8s.io/move":""}}}' ``` ### CSO and CSPO variables preparation From 937bb0f536019774d026e18ceb1d356aac5fadcd Mon Sep 17 00:00:00 2001 From: "michal.gubricky" Date: Mon, 26 Feb 2024 12:49:48 +0100 Subject: [PATCH 7/8] Get rid off the spec.CloudName field from the OpenStackNodeImageRelease resource Signed-off-by: michal.gubricky --- api/v1alpha1/conditions_const.go | 6 -- .../openstacknodeimagerelease_types.go | 3 - ...k.x-k8s.io_openstacknodeimagereleases.yaml | 6 -- ...openstackclusterstackrelease_controller.go | 57 +------------------ ...tackclusterstackrelease_controller_test.go | 57 +------------------ .../openstacknodeimagerelease_controller.go | 19 ++++++- ...enstacknodeimagerelease_controller_test.go | 16 +++--- 7 files changed, 28 insertions(+), 136 deletions(-) diff --git a/api/v1alpha1/conditions_const.go b/api/v1alpha1/conditions_const.go index 321a0c8e..d2d0a5bb 100644 --- a/api/v1alpha1/conditions_const.go +++ b/api/v1alpha1/conditions_const.go @@ -48,9 +48,6 @@ const ( ) const ( - // CloudNameAvailableCondition is used when cloud name is available. - CloudNameAvailableCondition = "CloudNameAvailable" - // CloudAvailableCondition is used when cloud is available. CloudAvailableCondition = "CloudAvailable" @@ -59,9 +56,6 @@ const ( // SecretNotFoundReason is used when the secret specified by the user is not found. SecretNotFoundReason = "SecretNotFound" - - // IssueWithSecretReason is used when getting the key-value pair from the secret failed. - IssueWithSecretReason = "IssueWithSecret" ) const ( diff --git a/api/v1alpha1/openstacknodeimagerelease_types.go b/api/v1alpha1/openstacknodeimagerelease_types.go index f2c3d9bb..61ffd05b 100644 --- a/api/v1alpha1/openstacknodeimagerelease_types.go +++ b/api/v1alpha1/openstacknodeimagerelease_types.go @@ -28,9 +28,6 @@ import ( // OpenStackNodeImageReleaseSpec defines the desired state of OpenStackNodeImageRelease. type OpenStackNodeImageReleaseSpec struct { - // CloudName is the name of the cloud to use from the cloud's secret. - // +kubebuilder:validation:MinLength=1 - CloudName string `json:"cloudName"` // IdentityRef is a reference to a identity to be used when reconciling this cluster IdentityRef *apiv1alpha7.OpenStackIdentityReference `json:"identityRef"` // Image represents options used to upload an image diff --git a/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstacknodeimagereleases.yaml b/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstacknodeimagereleases.yaml index 4d22c053..f4e2f901 100644 --- a/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstacknodeimagereleases.yaml +++ b/config/crd/bases/infrastructure.clusterstack.x-k8s.io_openstacknodeimagereleases.yaml @@ -52,11 +52,6 @@ spec: description: OpenStackNodeImageReleaseSpec defines the desired state of OpenStackNodeImageRelease. properties: - cloudName: - description: CloudName is the name of the cloud to use from the cloud's - secret. - minLength: 1 - type: string identityRef: description: IdentityRef is a reference to a identity to be used when reconciling this cluster @@ -129,7 +124,6 @@ spec: - url type: object required: - - cloudName - identityRef - image type: object diff --git a/internal/controller/openstackclusterstackrelease_controller.go b/internal/controller/openstackclusterstackrelease_controller.go index 8b735c59..e7a1c0c4 100644 --- a/internal/controller/openstackclusterstackrelease_controller.go +++ b/internal/controller/openstackclusterstackrelease_controller.go @@ -29,7 +29,6 @@ import ( githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/release" apiv1alpha1 "github.com/sovereignCloudStack/cluster-stack-provider-openstack/api/v1alpha1" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -61,7 +60,6 @@ type NodeImages struct { } const ( - cloudNameSecretKey = "cloudName" metadataFileName = "metadata.yaml" nodeImagesFileName = "node-images.yaml" waitForOpenStackNodeImageReleasesBecomeReady = 30 * time.Second @@ -176,32 +174,8 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context, } osnirName := fmt.Sprintf("%s-%s-%s", nameWithoutVersion, openStackNodeImage.CreateOpts.Name, nodeImageVersion) - cloudName, err := r.getCloudNameFromSecret(ctx, openstackclusterstackrelease.Namespace, openstackclusterstackrelease.Spec.IdentityRef.Name) - if err != nil { - if apierrors.IsNotFound(err) { - conditions.MarkFalse(openstackclusterstackrelease, - apiv1alpha1.CloudNameAvailableCondition, - apiv1alpha1.SecretNotFoundReason, - clusterv1beta1.ConditionSeverityError, - err.Error(), - ) - record.Warnf(openstackclusterstackrelease, "SecretNotFound", err.Error()) - logger.Error(err, "failed to get secret") - return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil - } - conditions.MarkFalse(openstackclusterstackrelease, - apiv1alpha1.CloudNameAvailableCondition, - apiv1alpha1.IssueWithSecretReason, - clusterv1beta1.ConditionSeverityWarning, - err.Error(), - ) - record.Warnf(openstackclusterstackrelease, "IssueWithSecret", err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to get cloud name from secret: %w", err) - } - - conditions.MarkTrue(openstackclusterstackrelease, apiv1alpha1.CloudNameAvailableCondition) - if err := r.createOrUpdateOpenStackNodeImageRelease(ctx, openstackclusterstackrelease, osnirName, cloudName, openStackNodeImage, ownerRef); err != nil { + if err := r.createOrUpdateOpenStackNodeImageRelease(ctx, openstackclusterstackrelease, osnirName, openStackNodeImage, ownerRef); err != nil { return ctrl.Result{}, fmt.Errorf("failed to create or update OpenStackNodeImageRelease %s/%s: %w", openstackclusterstackrelease.Namespace, osnirName, err) } } @@ -244,7 +218,7 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context, return ctrl.Result{}, nil } -func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImageRelease(ctx context.Context, openstackclusterstackrelease *apiv1alpha1.OpenStackClusterStackRelease, osnirName, cloudName string, openStackNodeImage *apiv1alpha1.OpenStackNodeImage, ownerRef *metav1.OwnerReference) error { +func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImageRelease(ctx context.Context, openstackclusterstackrelease *apiv1alpha1.OpenStackClusterStackRelease, osnirName string, openStackNodeImage *apiv1alpha1.OpenStackNodeImage, ownerRef *metav1.OwnerReference) error { openStackNodeImageRelease := &apiv1alpha1.OpenStackNodeImageRelease{} err := r.Get(ctx, types.NamespacedName{Name: osnirName, Namespace: openstackclusterstackrelease.Namespace}, openStackNodeImageRelease) @@ -276,7 +250,6 @@ func (r *OpenStackClusterStackReleaseReconciler) createOrUpdateOpenStackNodeImag } openStackNodeImageRelease.SetOwnerReferences([]metav1.OwnerReference{*ownerRef}) openStackNodeImageRelease.Spec.Image = openStackNodeImage - openStackNodeImageRelease.Spec.CloudName = cloudName openStackNodeImageRelease.Spec.IdentityRef = openstackclusterstackrelease.Spec.IdentityRef if err := r.Create(ctx, openStackNodeImageRelease); err != nil { @@ -376,32 +349,6 @@ func cutOpenStackClusterStackReleaseVersionFromReleaseTag(releaseTag string) (st return fmt.Sprintf("%s-%s-%s-%s", v[0], v[1], v[2], v[3]), nil } -func (r *OpenStackClusterStackReleaseReconciler) getCloudNameFromSecret(ctx context.Context, secretNamespace, secretName string) (string, error) { - var cloudName string - emptyCloudName := "" - defaultCloudName := "openstack" - - secret := &corev1.Secret{} - err := r.Get(ctx, types.NamespacedName{ - Namespace: secretNamespace, - Name: secretName, - }, secret) - if err != nil { - return emptyCloudName, fmt.Errorf("failed to get secret %s in namespace %s: %w", secretName, secretNamespace, err) - } - - content, ok := secret.Data[cloudNameSecretKey] - if !ok { - return defaultCloudName, nil - } - - if err := yaml.Unmarshal(content, &cloudName); err != nil { - return emptyCloudName, fmt.Errorf("failed to unmarshal cloudName stored in secret %s: %w", secretName, err) - } - - return cloudName, nil -} - // SetupWithManager sets up the controller with the Manager. func (r *OpenStackClusterStackReleaseReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/internal/controller/openstackclusterstackrelease_controller_test.go b/internal/controller/openstackclusterstackrelease_controller_test.go index a58a5c35..8159e9e8 100644 --- a/internal/controller/openstackclusterstackrelease_controller_test.go +++ b/internal/controller/openstackclusterstackrelease_controller_test.go @@ -341,10 +341,9 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) { Client: client, } - cloudName, err := r.getCloudNameFromSecret(context.TODO(), secretNamespace, secretName) assert.NoError(t, err) - err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-osnir", cloudName, openStackNodeImage, ownerRef) + err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-osnir", openStackNodeImage, ownerRef) assert.NoError(t, err) osnir := &apiv1alpha1.OpenStackNodeImageRelease{} @@ -358,7 +357,6 @@ func TestCreateOpenStackNodeImageRelease(t *testing.T) { APIVersion: apiv1alpha1.GroupVersion.String(), }, Spec: apiv1alpha1.OpenStackNodeImageReleaseSpec{ - CloudName: "openstack", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", @@ -439,7 +437,6 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) { Namespace: "test-namespace", }, Spec: apiv1alpha1.OpenStackNodeImageReleaseSpec{ - CloudName: "test-cloudname", IdentityRef: &capoapiv1alpha7.OpenStackIdentityReference{ Kind: "Secret", Name: "supersecret", @@ -469,7 +466,7 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) { assert.NoError(t, err) assert.Equal(t, ownerRef.UID, osnir.OwnerReferences[0].UID) - err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-update-osnir", "test-cloud-name", openStackNodeImage, newOwnerRef) + err = r.createOrUpdateOpenStackNodeImageRelease(context.TODO(), openstackclusterstackrelease, "test-update-osnir", openStackNodeImage, newOwnerRef) assert.NoError(t, err) err = client.Get(context.TODO(), types.NamespacedName{Name: "test-update-osnir", Namespace: "test-namespace"}, osnir) @@ -484,56 +481,6 @@ func TestUpdateOpenStackNodeImageRelease(t *testing.T) { assert.NoError(t, err) } -func TestGetCloudNameFromSecret(t *testing.T) { - client := fake.NewClientBuilder().Build() - - secretName := "supersecret" - secretNamespace := "test-namespace" - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - Namespace: secretNamespace, - }, - Type: corev1.SecretTypeOpaque, - } - err := client.Create(context.TODO(), secret) - assert.NoError(t, err) - - r := &OpenStackClusterStackReleaseReconciler{ - Client: client, - } - - cloudName, err := r.getCloudNameFromSecret(context.TODO(), secretNamespace, secretName) - expectedCloudName := "openstack" - - assert.NoError(t, err) - assert.Equal(t, expectedCloudName, cloudName) - - err = client.Delete(context.TODO(), secret) - assert.NoError(t, err) -} - -func TestGetCloudNameFromSecretNotFound(t *testing.T) { - client := fake.NewClientBuilder().Build() - - r := &OpenStackClusterStackReleaseReconciler{ - Client: client, - } - - secretName := "nonexistent-secret" - secretNamespace := "nonexistent-namespace" - expectedError := "secrets \"nonexistent-secret\" not found" - - cloudName, err := r.getCloudNameFromSecret(context.TODO(), secretNamespace, secretName) - - expectedErrorMessage := fmt.Sprintf("failed to get secret %s in namespace %s: %v", secretName, secretNamespace, expectedError) - - assert.Error(t, err) - assert.True(t, apierrors.IsNotFound(err)) - assert.Equal(t, "", cloudName) - assert.EqualError(t, err, expectedErrorMessage) -} - var _ = Describe("OpenStackClusterStackRelease controller", func() { Context("OpenStackClusterStackRelease controller test", func() { const openstackclusterstackreleasename = "test-ocsr" diff --git a/internal/controller/openstacknodeimagerelease_controller.go b/internal/controller/openstacknodeimagerelease_controller.go index 87972afa..93033de1 100644 --- a/internal/controller/openstacknodeimagerelease_controller.go +++ b/internal/controller/openstacknodeimagerelease_controller.go @@ -50,6 +50,7 @@ type OpenStackNodeImageReleaseReconciler struct { } const ( + cloudNameSecretKey = "cloudName" cloudsSecretKey = "clouds.yaml" waitForImageBecomeActive = 30 * time.Second ) @@ -95,7 +96,7 @@ func (r *OpenStackNodeImageReleaseReconciler) Reconcile(ctx context.Context, req }() // Get OpenStack cloud config from sercet - cloud, err := r.getCloudFromSecret(ctx, openstacknodeimagerelease.Namespace, openstacknodeimagerelease.Spec.IdentityRef.Name, openstacknodeimagerelease.Spec.CloudName) + cloud, err := r.getCloudFromSecret(ctx, openstacknodeimagerelease.Namespace, openstacknodeimagerelease.Spec.IdentityRef.Name) if err != nil { if apierrors.IsNotFound(err) { conditions.MarkFalse(openstacknodeimagerelease, @@ -291,9 +292,11 @@ func (r *OpenStackNodeImageReleaseReconciler) Reconcile(ctx context.Context, req return ctrl.Result{}, nil } -func (r *OpenStackNodeImageReleaseReconciler) getCloudFromSecret(ctx context.Context, secretNamespace, secretName, cloudName string) (clientconfig.Cloud, error) { +func (r *OpenStackNodeImageReleaseReconciler) getCloudFromSecret(ctx context.Context, secretNamespace, secretName string) (clientconfig.Cloud, error) { var clouds clientconfig.Clouds emptyCloud := clientconfig.Cloud{} + var cloudName string + defaultCloudName := "openstack" secret := &corev1.Secret{} err := r.Get(ctx, types.NamespacedName{ @@ -303,7 +306,17 @@ func (r *OpenStackNodeImageReleaseReconciler) getCloudFromSecret(ctx context.Con if err != nil { return emptyCloud, fmt.Errorf("failed to get secret %s in namespace %s: %w", secretName, secretNamespace, err) } - content, ok := secret.Data[cloudsSecretKey] + + content, ok := secret.Data[cloudNameSecretKey] + if !ok { + cloudName = defaultCloudName + } else { + if err := yaml.Unmarshal(content, &cloudName); err != nil { + return emptyCloud, fmt.Errorf("failed to unmarshal cloudName stored in secret %s: %w", secretName, err) + } + } + + content, ok = secret.Data[cloudsSecretKey] if !ok { return emptyCloud, fmt.Errorf("OpenStack credentials secret %s did not contain key %s", secretName, cloudsSecretKey) } diff --git a/internal/controller/openstacknodeimagerelease_controller_test.go b/internal/controller/openstacknodeimagerelease_controller_test.go index 2d1baf5c..841faaf6 100644 --- a/internal/controller/openstacknodeimagerelease_controller_test.go +++ b/internal/controller/openstacknodeimagerelease_controller_test.go @@ -40,7 +40,6 @@ func TestGetCloudFromSecret(t *testing.T) { secretName := "test-secret" secretNamespace := "test-namespace" - cloudName := "openstack" cloudsYAML := ` clouds: openstack: @@ -69,7 +68,7 @@ clouds: Client: client, } - cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName, cloudName) + cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName) expectedCloud := clientconfig.Cloud{ AuthInfo: &clientconfig.AuthInfo{ @@ -99,9 +98,8 @@ func TestGetCloudFromSecretNotFound(t *testing.T) { secretName := "nonexistent-secret" secretNamespace := "nonexistent-namespace" expectedError := "secrets \"nonexistent-secret\" not found" - cloudName := "nonexistent-cloud" - cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName, cloudName) + cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName) expectedErrorMessage := fmt.Sprintf("failed to get secret %s in namespace %s: %v", secretName, secretNamespace, expectedError) @@ -120,7 +118,6 @@ func TestGetCloudFromSecretMissingCloudsSecretKey(t *testing.T) { secretName := "test-secret" secretNamespace := "test-namespace" - cloudName := "openstack" // Create a secret with the bad cloudsSecretKey. secret := &corev1.Secret{ @@ -134,7 +131,7 @@ func TestGetCloudFromSecretMissingCloudsSecretKey(t *testing.T) { err := client.Create(context.TODO(), secret) assert.NoError(t, err) - cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName, cloudName) + cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName) assert.Error(t, err) assert.EqualError(t, err, fmt.Sprintf("OpenStack credentials secret %s did not contain key %s", secretName, cloudsSecretKey)) @@ -172,13 +169,16 @@ clouds: Name: secretName, Namespace: secretNamespace, }, - Data: map[string][]byte{cloudsSecretKey: []byte(cloudsYAML)}, + Data: map[string][]byte{ + cloudsSecretKey: []byte(cloudsYAML), + cloudNameSecretKey: []byte(cloudName), + }, Type: corev1.SecretTypeOpaque, } err := client.Create(context.TODO(), secret) assert.NoError(t, err) - cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName, cloudName) + cloud, err := r.getCloudFromSecret(context.TODO(), secretNamespace, secretName) assert.Error(t, err) assert.EqualError(t, err, fmt.Sprintf("failed to find cloud %s in %s", cloudName, cloudsSecretKey)) From 57acf4de775ebf545130843b20e443de4340c3ad Mon Sep 17 00:00:00 2001 From: "michal.gubricky" Date: Tue, 27 Feb 2024 14:03:51 +0100 Subject: [PATCH 8/8] Move defaultCloudName into const Signed-off-by: michal.gubricky --- internal/controller/openstacknodeimagerelease_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/openstacknodeimagerelease_controller.go b/internal/controller/openstacknodeimagerelease_controller.go index 93033de1..73c984d3 100644 --- a/internal/controller/openstacknodeimagerelease_controller.go +++ b/internal/controller/openstacknodeimagerelease_controller.go @@ -50,6 +50,7 @@ type OpenStackNodeImageReleaseReconciler struct { } const ( + defaultCloudName = "openstack" cloudNameSecretKey = "cloudName" cloudsSecretKey = "clouds.yaml" waitForImageBecomeActive = 30 * time.Second @@ -296,7 +297,6 @@ func (r *OpenStackNodeImageReleaseReconciler) getCloudFromSecret(ctx context.Con var clouds clientconfig.Clouds emptyCloud := clientconfig.Cloud{} var cloudName string - defaultCloudName := "openstack" secret := &corev1.Secret{} err := r.Get(ctx, types.NamespacedName{