diff --git a/internal/controllers/capiprovider_controller.go b/internal/controllers/capiprovider_controller.go index baff3449..2d7c8090 100644 --- a/internal/controllers/capiprovider_controller.go +++ b/internal/controllers/capiprovider_controller.go @@ -74,12 +74,18 @@ func (r *CAPIProviderReconciler) sync(ctx context.Context, capiProvider *turtles sync.NewSecretMapperSync(ctx, r.Client, capiProvider), ) + defer r.patchStatus(ctx, capiProvider, &err) + if err := s.Sync(ctx); client.IgnoreNotFound(err) != nil { return ctrl.Result{}, err } defer s.Apply(ctx, &err) - return ctrl.Result{}, sync.PatchStatus(ctx, r.Client, capiProvider) + return ctrl.Result{}, nil +} + +func (r *CAPIProviderReconciler) patchStatus(ctx context.Context, capiProvider *turtlesv1.CAPIProvider, err *error) { + *err = sync.PatchStatus(ctx, r.Client, capiProvider) } // SetupWithManager sets up the controller with the Manager. diff --git a/internal/controllers/capiprovider_controller_test.go b/internal/controllers/capiprovider_controller_test.go index 7cfb8826..aa9cd4cd 100644 --- a/internal/controllers/capiprovider_controller_test.go +++ b/internal/controllers/capiprovider_controller_test.go @@ -25,6 +25,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha2" + "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/controller-runtime/pkg/client" . "sigs.k8s.io/controller-runtime/pkg/envtest/komega" ) @@ -123,7 +124,7 @@ var _ = Describe("Reconcile CAPIProvider", func() { }, })).To(Succeed()) - Expect(cl.Create(ctx, &corev1.Secret{ + secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "cc", GenerateName: "cc-", @@ -136,7 +137,8 @@ var _ = Describe("Reconcile CAPIProvider", func() { StringData: map[string]string{ "digitaloceancredentialConfig-accessToken": "token", }, - })).To(Succeed()) + } + Expect(cl.Create(ctx, secret)).To(Succeed()) Eventually(Update(provider, func() { provider.Spec.Features = &turtlesv1.Features{MachinePool: true} @@ -153,5 +155,36 @@ var _ = Describe("Reconcile CAPIProvider", func() { "DO_B64ENCODED_CREDENTIALS": []byte("dG9rZW4="), }))) + Eventually(func(g Gomega) { + g.Expect(testEnv.Get(ctx, client.ObjectKeyFromObject(provider), provider)).ToNot(HaveOccurred()) + g.Expect(conditions.IsTrue(provider, turtlesv1.RancherCredentialsSecretCondition)) + }).Should(Succeed()) + }) + + It("Should reflect missing infrastructure digitalocean provider credential secret in the status", func() { + provider := &turtlesv1.CAPIProvider{ObjectMeta: metav1.ObjectMeta{ + Name: "digitalocean", + Namespace: ns.Name, + }, Spec: turtlesv1.CAPIProviderSpec{ + Type: turtlesv1.Infrastructure, + }} + Expect(cl.Create(ctx, provider)).ToNot(HaveOccurred()) + + doSecret := objectFromKey(client.ObjectKeyFromObject(provider), &corev1.Secret{}) + Eventually(testEnv.GetAs(provider, &operatorv1.InfrastructureProvider{})).ShouldNot(BeNil()) + Eventually(testEnv.GetAs(provider, doSecret)).ShouldNot(BeNil()) + + Eventually(Update(provider, func() { + provider.Spec.Features = &turtlesv1.Features{MachinePool: true} + provider.Spec.Credentials = &turtlesv1.Credentials{ + RancherCloudCredential: "some-missing", + } + })).Should(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(testEnv.Get(ctx, client.ObjectKeyFromObject(provider), provider)).ToNot(HaveOccurred()) + g.Expect(conditions.IsFalse(provider, turtlesv1.RancherCredentialsSecretCondition)) + g.Expect(conditions.GetMessage(provider, turtlesv1.RancherCredentialsSecretCondition)).To(Equal("Credential keys missing: key not found: digitaloceancredentialConfig-accessToken")) + }).Should(Succeed()) }) }) diff --git a/internal/controllers/import_controller_test.go b/internal/controllers/import_controller_test.go index 5b6db481..fa0f8369 100644 --- a/internal/controllers/import_controller_test.go +++ b/internal/controllers/import_controller_test.go @@ -138,7 +138,7 @@ var _ = Describe("reconcile CAPI Cluster", func() { }) g.Expect(err).ToNot(HaveOccurred()) g.Expect(res.RequeueAfter).To(Equal(defaultRequeueDuration)) - }) + }).Should(Succeed()) }) It("should reconcile a CAPI cluster when rancher cluster doesn't exist", func() { diff --git a/internal/sync/provider_sync_test.go b/internal/sync/provider_sync_test.go index 0116873c..237e0cdf 100644 --- a/internal/sync/provider_sync_test.go +++ b/internal/sync/provider_sync_test.go @@ -138,7 +138,7 @@ var _ = Describe("Provider sync", func() { g.Expect(testEnv.Get(ctx, client.ObjectKeyFromObject(capiProvider), capiProvider)).To(Succeed()) g.Expect(conditions.Get(capiProvider, turtlesv1.LastAppliedConfigurationTime)).ToNot(BeNil()) g.Expect(conditions.Get(capiProvider, turtlesv1.LastAppliedConfigurationTime).LastTransitionTime.Second()).To(Equal(appliedCondition.LastTransitionTime.Second())) - }) + }).Should(Succeed()) s := sync.NewProviderSync(testEnv, capiProvider) @@ -158,6 +158,7 @@ var _ = Describe("Provider sync", func() { appliedCondition.LastTransitionTime.Time)).To(BeTrue()) }).Should(Succeed()) + Expect(testEnv.Get(ctx, client.ObjectKeyFromObject(capiProvider), capiProvider)).To(Succeed()) condition := conditions.Get(capiProvider, turtlesv1.LastAppliedConfigurationTime) Consistently(func(g Gomega) { @@ -167,7 +168,7 @@ var _ = Describe("Provider sync", func() { s.Apply(ctx, &err) g.Expect(testEnv.Get(ctx, client.ObjectKeyFromObject(capiProvider), capiProvider)).To(Succeed()) g.Expect(conditions.Get(capiProvider, turtlesv1.LastAppliedConfigurationTime)).To(Equal(condition)) - }, 5*time.Second) + }, 5*time.Second).Should(Succeed()) }) It("Should individually sync every provider", func() { @@ -208,7 +209,7 @@ var _ = Describe("Provider sync", func() { g.Expect(capiProviderDuplicate.Status.Conditions).To(HaveLen(1)) g.Expect(conditions.IsTrue(capiProviderDuplicate, turtlesv1.LastAppliedConfigurationTime)).To(BeTrue()) g.Expect(conditions.Get(capiProviderDuplicate, turtlesv1.LastAppliedConfigurationTime).LastTransitionTime.Second()).To(Equal(time.Now().UTC().Second())) - }) + }).Should(Succeed()) // Provider manifest should be created and phase set to provisioning Eventually(func(g Gomega) { diff --git a/internal/sync/secret_mapper_sync.go b/internal/sync/secret_mapper_sync.go index 9d37646e..27c6476c 100644 --- a/internal/sync/secret_mapper_sync.go +++ b/internal/sync/secret_mapper_sync.go @@ -98,7 +98,7 @@ var ( ) var ( - missingKey = "Credential keys missing: %w" + missingKey = "Credential keys missing: %s" missingSource = "Rancher Credentials secret named %s was not located" ) diff --git a/internal/sync/secret_mapper_sync_test.go b/internal/sync/secret_mapper_sync_test.go index 3337bcf2..3428a04d 100644 --- a/internal/sync/secret_mapper_sync_test.go +++ b/internal/sync/secret_mapper_sync_test.go @@ -146,7 +146,7 @@ var _ = Describe("SecretMapperSync get", func() { g.Expect(syncer.Get(context.Background())).ToNot(HaveOccurred()) g.Expect(syncer.RancherSecret.Annotations).To(Equal(rancherSecret.Annotations)) g.Expect(syncer.RancherSecret.Name).To(Equal(rancherSecret.Name)) - }) + }).Should(Succeed()) }) It("should ignore similarly named secret for a different driver", func() { @@ -165,12 +165,13 @@ var _ = Describe("SecretMapperSync get", func() { } Eventually(func(g Gomega) { - g.Expect(syncer.Get(context.Background())).To(HaveOccurred()) + err = syncer.Get(context.Background()) + g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring("unable to locate rancher secret with name")) g.Expect(conditions.Get(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).ToNot(BeNil()) g.Expect(conditions.IsFalse(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).To(BeTrue()) g.Expect(conditions.GetMessage(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).To(Equal("Rancher Credentials secret named test-rancher-secret was not located")) - }) + }).Should(Succeed()) }) It("should get the source Rancher secret pointed by ref", func() { @@ -190,7 +191,7 @@ var _ = Describe("SecretMapperSync get", func() { g.Expect(syncer.Get(context.Background())).NotTo(HaveOccurred()) g.Expect(syncer.RancherSecret.Name).To(Equal(customRancherSecret.Name)) g.Expect(syncer.RancherSecret.Namespace).To(Equal(customRancherSecret.Namespace)) - }) + }).Should(Succeed()) }) It("should handle unexisting secret pointer by ref", func() { @@ -200,18 +201,19 @@ var _ = Describe("SecretMapperSync get", func() { }} Expect(testEnv.Client.Create(ctx, secret)).To(Succeed()) - syncer := sync.SecretMapperSync{ + syncer := &sync.SecretMapperSync{ SecretSync: sync.NewSecretSync(testEnv.Client, capiProviderWithRancherRef).(*sync.SecretSync), RancherSecret: sync.SecretMapperSync{}.GetSecret(capiProviderWithRancherRef), } Eventually(func(g Gomega) { - g.Expect(syncer.Get(context.Background())).NotTo(HaveOccurred()) + err = syncer.Get(context.Background()) + g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring("unable to locate rancher secret with name")) g.Expect(conditions.Get(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).ToNot(BeNil()) g.Expect(conditions.IsFalse(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).To(BeTrue()) g.Expect(conditions.GetMessage(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).To(Equal(fmt.Sprintf("Rancher Credentials secret named %s:secret-name was not located", ns.Name))) - }) + }).Should(Succeed()) }) It("should handle when the source Rancher secret is not found", func() { @@ -227,12 +229,13 @@ var _ = Describe("SecretMapperSync get", func() { } Eventually(func(g Gomega) { - g.Expect(syncer.Get(context.Background())).To(HaveOccurred()) + err = syncer.Get(context.Background()) + g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring("unable to locate rancher secret with name")) g.Expect(conditions.Get(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).ToNot(BeNil()) g.Expect(conditions.IsFalse(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).To(BeTrue()) g.Expect(conditions.GetMessage(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).To(Equal("Rancher Credentials secret named test-rancher-secret was not located")) - }) + }).Should(Succeed()) }) It("should not error when the destination secret is not created yet", func() { @@ -247,7 +250,7 @@ var _ = Describe("SecretMapperSync get", func() { g.Expect(syncer.Get(context.Background())).ToNot(HaveOccurred()) g.Expect(syncer.RancherSecret.Annotations).To(Equal(rancherSecret.Annotations)) g.Expect(syncer.RancherSecret.Name).To(Equal(rancherSecret.Name)) - }) + }).Should(Succeed()) }) It("should point to the right initial secret", func() { @@ -275,7 +278,7 @@ var _ = Describe("SecretMapperSync get", func() { Eventually(func(g Gomega) { g.Expect(syncer.Sync(context.Background())).To(BeNil()) g.Expect(syncer.SecretSync.Secret.StringData).To(HaveLen(0)) - }) + }).Should(Succeed()) }) It("provider requirements azure", func() { @@ -301,7 +304,7 @@ var _ = Describe("SecretMapperSync get", func() { "AZURE_TENANT_ID": "", "AZURE_SUBSCRIPTION_ID_B64": "", })) - }) + }).Should(Succeed()) }) It("provider requirements aws", func() { @@ -323,7 +326,7 @@ var _ = Describe("SecretMapperSync get", func() { "AWS_ACCESS_KEY_ID": "", "AWS_SECRET_ACCESS_KEY": "", })) - }) + }).Should(Succeed()) }) It("provider requirements gcp", func() { @@ -342,7 +345,7 @@ var _ = Describe("SecretMapperSync get", func() { g.Expect(syncer.Secret.StringData).To(Equal(map[string]string{ "GCP_B64ENCODED_CREDENTIALS": "", })) - }) + }).Should(Succeed()) }) It("provider requirements digitalocean", func() { capiProvider.Spec.Name = "digitalocean" @@ -361,7 +364,7 @@ var _ = Describe("SecretMapperSync get", func() { "DO_B64ENCODED_CREDENTIALS": "", "DIGITALOCEAN_ACCESS_TOKEN": "", })) - }) + }).Should(Succeed()) }) It("provider requirements vsphere", func() { capiProvider.Spec.Name = "vsphere" @@ -379,7 +382,7 @@ var _ = Describe("SecretMapperSync get", func() { "VSPHERE_PASSWORD": "", "VSPHERE_USERNAME": "", })) - }) + }).Should(Succeed()) }) It("prepare aws secret", func() { @@ -407,6 +410,6 @@ var _ = Describe("SecretMapperSync get", func() { g.Expect(conditions.Get(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).ToNot(BeNil()) g.Expect(conditions.IsTrue(syncer.Source, turtlesv1.RancherCredentialsSecretCondition)).To(BeTrue()) - }) + }).Should(Succeed()) }) })