update according to review comments

Signed-off-by: Artem Bortnikov <[email protected]>
K0rdent · Dec 30, 2024 · 58f8620 · 58f8620
1 parent 104de0f
commit 58f8620
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)
 }