From 58f8620e0c43ac2097b7961486d2b8cbe211947f Mon Sep 17 00:00:00 2001 From: Artem Bortnikov Date: Mon, 30 Dec 2024 17:06:59 +0200 Subject: [PATCH] update according to review comments Signed-off-by: Artem Bortnikov --- .../clusterdeployment_controller_test.go | 7 ++++--- internal/controller/suite_test.go | 2 +- .../controller/template_controller_test.go | 18 +----------------- internal/helm/actor.go | 4 ++++ 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/internal/controller/clusterdeployment_controller_test.go b/internal/controller/clusterdeployment_controller_test.go index 96053067..1ce6b7de 100644 --- a/internal/controller/clusterdeployment_controller_test.go +++ b/internal/controller/clusterdeployment_controller_test.go @@ -559,6 +559,7 @@ var _ = Describe("ClusterDeployment Controller", func() { HaveField("Status", metav1.ConditionTrue), HaveField("Reason", hmc.SucceededReason), ), + // TODO (#852 brongineer): add corresponding resources with expected state for successful reconciliation // SatisfyAll( // HaveField("Type", hmc.FetchServicesStatusSuccessCondition), // HaveField("Status", metav1.ConditionTrue), @@ -574,17 +575,17 @@ var _ = Describe("ClusterDeployment Controller", func() { }) }) - // TODO (brongineer): Add test for ClusterDeployment reconciliation with Azure credentials + // TODO (#852 brongineer): Add test for ClusterDeployment reconciliation with Azure credentials PIt("should reconcile ClusterDeployment with Azure credentials", func() { // TBD }) - // TODO (brongineer): Add tests for ClusterDeployment reconciliation with other providers' credentials + // TODO (#852 brongineer): Add tests for ClusterDeployment reconciliation with other providers' credentials PIt("should reconcile ClusterDeployment with XXX credentials", func() { // TBD }) - // TODO (brongineer): Add test for ClusterDeployment deletion + // TODO (#852 brongineer): Add test for ClusterDeployment deletion PIt("should reconcile ClusterDeployment deletion", func() { // TBD }) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index a7cc8af4..161a9553 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -286,7 +286,7 @@ func seedClusterScopedResources(ctx context.Context, k8sClient client.Client) er } Expect(k8sClient.Create(ctx, management)).To(Succeed()) management.Status = hmcmirantiscomv1alpha1.ManagementStatus{ - AvailableProviders: []string{someProviderName, otherProviderName, "infrastructure-aws"}, + AvailableProviders: []string{someProviderName, otherProviderName}, CAPIContracts: map[string]hmcmirantiscomv1alpha1.CompatibilityContracts{someProviderName: {capiVersion: someExposedContract}, otherProviderName: {capiVersion: otherExposedContract}}, } Expect(k8sClient.Status().Update(ctx, management)).To(Succeed()) diff --git a/internal/controller/template_controller_test.go b/internal/controller/template_controller_test.go index 5ec32b35..1520589f 100644 --- a/internal/controller/template_controller_test.go +++ b/internal/controller/template_controller_test.go @@ -256,22 +256,6 @@ var _ = Describe("Template Controller", func() { return nil }).WithTimeout(timeout).WithPolling(interval).Should(Succeed()) - // By("Creating a management cluster object with proper required versions in status") - // // must set status here since it's controller by another ctrl - // mgmt := &hmcmirantiscomv1alpha1.Management{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: mgmtName, - // }, - // Spec: hmcmirantiscomv1alpha1.ManagementSpec{ - // Release: "test-release", - // }, - // } - // Expect(k8sClient.Create(ctx, mgmt)).To(Succeed()) - // mgmt.Status = hmcmirantiscomv1alpha1.ManagementStatus{ - // AvailableProviders: []string{someProviderName, otherProviderName}, - // CAPIContracts: map[string]hmcmirantiscomv1alpha1.CompatibilityContracts{someProviderName: {capiVersion: someExposedContract}, otherProviderName: {capiVersion: otherExposedContract}}, - // } - // Expect(k8sClient.Status().Update(ctx, mgmt)).To(Succeed()) mgmt := &hmcmirantiscomv1alpha1.Management{} key := client.ObjectKey{Name: mgmtName} Expect(k8sClient.Get(ctx, key, mgmt)).To(Succeed()) @@ -282,7 +266,7 @@ var _ = Describe("Template Controller", func() { return err } - if l := len(mgmt.Status.AvailableProviders); l != 3 { + if l := len(mgmt.Status.AvailableProviders); l != 2 { return fmt.Errorf("expected .status.availableProviders length to be exactly 2, got %d", l) } if l := len(mgmt.Status.CAPIContracts); l != 2 { diff --git a/internal/helm/actor.go b/internal/helm/actor.go index cab6b545..40c2caa3 100644 --- a/internal/helm/actor.go +++ b/internal/helm/actor.go @@ -16,6 +16,7 @@ package helm import ( "context" + "errors" sourcev1 "github.com/fluxcd/source-controller/api/v1" "helm.sh/helm/v3/pkg/action" @@ -39,6 +40,9 @@ func NewActor(config *rest.Config, mapper apimeta.RESTMapper) *Actor { } func (*Actor) DownloadChartFromArtifact(ctx context.Context, artifact *sourcev1.Artifact) (*chart.Chart, error) { + if artifact == nil { + return nil, errors.New("helm chart artifact is not ready yet") + } return DownloadChart(ctx, artifact.URL, artifact.Digest) }