🌱 Fix linting, tests and prepare for merge

Fix linting in new clusteraddon-related files that we changed in the feature branch. Fixing unit tests. Prepare feature branch to merge by going through changes of PR and validating them. Fixing some small stuff, as well as changing unnecessary or temporary changes back to the state of the main branch. Signed-off-by: janiskemper <[email protected]>
SovereignCloudStack · Jun 26, 2024 · 0fb787c · 0fb787c
1 parent c6cfdfd
commit 0fb787c
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 2,357 deletions.
diff --git a/Tiltfile b/Tiltfile
@@ -61,8 +61,9 @@ def deploy_capi():
                 patch_args_with_extra_args("capi-kubeadm-bootstrap-system", "capi-kubeadm-bootstrap-controller-manager", kb_extra_args)
 
 def deploy_capd():
-    yaml = './capd.yaml'
-    cmd = "kubectl apply -f capd.yaml"
+    version = settings.get("capi_version")
+    capd_uri = "https://github.com/kubernetes-sigs/cluster-api/releases/download/{}/infrastructure-components-development.yaml".format(version)
+    cmd = "curl -sSL {} | {} | kubectl apply -f -".format(capd_uri, envsubst_cmd)
     local(cmd, quiet = True)
 
 def prepare_environment():
@@ -211,12 +212,6 @@ def deploy_cso():
         labels = ["CSO"],
     )
 
-def deploy_capd():
-    yaml = './capd.yaml'
-    cmd = "kubectl apply -f capd.yaml"
-    local(cmd, quiet = True)
-
-
 def clusterstack():
     k8s_resource(objects = ["clusterstack:clusterstack"], new_name = "clusterstack", labels = ["CLUSTERSTACK"])
 

diff --git a/capd.yaml b/capd.yaml
diff --git a/cmd/main.go b/cmd/main.go
@@ -44,7 +44,6 @@ import (
 	runtimecatalog "sigs.k8s.io/cluster-api/exp/runtime/catalog"
 	runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
 	"sigs.k8s.io/cluster-api/exp/runtime/server"
-	dockerv1beta1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
@@ -67,7 +66,6 @@ func init() {
 
 	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
 	utilruntime.Must(csov1alpha1.AddToScheme(scheme))
-	utilruntime.Must(dockerv1beta1.AddToScheme(scheme))
 	utilruntime.Must(clusterv1.AddToScheme(scheme))
 	//+kubebuilder:scaffold:scheme
 }
@@ -227,7 +225,7 @@ func main() {
 		CertDir: hookCertDir,
 	})
 	if err != nil {
-		setupLog.Error(err, "error creating webhook server")
+		setupLog.Error(err, "error creating hook server")
 		os.Exit(1)
 	}
 

diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml
@@ -20,55 +20,55 @@ patchesStrategicMerge:
   - webhookcainjection_patch.yaml
   - manager_pull_policy.yaml
 vars:
-- name: CERTIFICATE_NAMESPACE  # namespace of the certificate CR
-  objref:
-    kind: Certificate
-    group: cert-manager.io
-    version: v1
-    name: serving-cert  # this name should match the one in certificate.yaml
-  fieldref:
-    fieldpath: metadata.namespace
-- name: HOOK_SERVER_CERTIFICATE_NAMESPACE  # namespace of the certificate CR
-  objref:
-    kind: Certificate
-    group: cert-manager.io
-    version: v1
-    name: hook-server-server-cert  # this name should match the one in certificate.yaml
-  fieldref:
-    fieldpath: metadata.namespace
-- name: CERTIFICATE_NAME
-  objref:
-    kind: Certificate
-    group: cert-manager.io
-    version: v1
-    name: serving-cert  # this name should match the one in certificate.yaml
-- name: HOOK_SERVER_CERTIFICATE_NAME
-  objref:
-    kind: Certificate
-    group: cert-manager.io
-    version: v1
-    name: hook-server-server-cert  # this name should match the one in certificate.yaml
-- name: SERVICE_NAMESPACE  # namespace of the service
-  objref:
-    kind: Service
-    version: v1
-    name: webhook-service
-  fieldref:
-    fieldpath: metadata.namespace
-- name: HOOK_SERVER_SERVICE_NAMESPACE  # namespace of the service
-  objref:
-    kind: Service
-    version: v1
-    name: hook-server-svc
-  fieldref:
-    fieldpath: metadata.namespace
-- name: SERVICE_NAME
-  objref:
-    kind: Service
-    version: v1
-    name: webhook-service
-- name: HOOK_SERVER_SERVICE_NAME
-  objref:
-    kind: Service
-    version: v1
-    name: hook-server-svc
+  - name: CERTIFICATE_NAMESPACE  # namespace of the certificate CR
+    objref:
+      kind: Certificate
+      group: cert-manager.io
+      version: v1
+      name: serving-cert  # this name should match the one in certificate.yaml
+    fieldref:
+      fieldpath: metadata.namespace
+  - name: HOOK_SERVER_CERTIFICATE_NAMESPACE  # namespace of the certificate CR
+    objref:
+      kind: Certificate
+      group: cert-manager.io
+      version: v1
+      name: hook-server-server-cert  # this name should match the one in certificate.yaml
+    fieldref:
+      fieldpath: metadata.namespace
+  - name: CERTIFICATE_NAME
+    objref:
+      kind: Certificate
+      group: cert-manager.io
+      version: v1
+      name: serving-cert  # this name should match the one in certificate.yaml
+  - name: HOOK_SERVER_CERTIFICATE_NAME
+    objref:
+      kind: Certificate
+      group: cert-manager.io
+      version: v1
+      name: hook-server-server-cert  # this name should match the one in certificate.yaml
+  - name: SERVICE_NAMESPACE  # namespace of the service
+    objref:
+      kind: Service
+      version: v1
+      name: webhook-service
+    fieldref:
+      fieldpath: metadata.namespace
+  - name: HOOK_SERVER_SERVICE_NAMESPACE  # namespace of the service
+    objref:
+      kind: Service
+      version: v1
+      name: hook-server-svc
+    fieldref:
+      fieldpath: metadata.namespace
+  - name: SERVICE_NAME
+    objref:
+      kind: Service
+      version: v1
+      name: webhook-service
+  - name: HOOK_SERVER_SERVICE_NAME
+    objref:
+      kind: Service
+      version: v1
+      name: hook-server-svc
diff --git a/internal/controller/clusteraddon_controller.go b/internal/controller/clusteraddon_controller.go
@@ -74,6 +74,11 @@ import (
 
 const clusterAddonNamespace = "kube-system"
 
+const (
+	beforeClusterUpgradeHook     = "BeforeClusterUpgrade"
+	afterControlPlaneInitialized = "AfterControlPlaneInitialized"
+)
+
 // RestConfigSettings contains Kubernetes rest config settings.
 type RestConfigSettings struct {
 	QPS   float32
@@ -344,8 +349,8 @@ func (r *ClusterAddonReconciler) Reconcile(ctx context.Context, req reconcile.Re
 	if clusterAddon.Spec.ClusterStack != cluster.Spec.Topology.Class && oldRelease != nil && oldRelease.Meta.Versions.Kubernetes == releaseAsset.Meta.Versions.Kubernetes {
 		if clusterAddon.Spec.Version != releaseAsset.Meta.Versions.Components.ClusterAddon {
 			if clusterAddon.Status.Ready || len(clusterAddon.Status.Stages) == 0 {
-				clusterAddon.Status.Stages = make([]csov1alpha1.StageStatus, len(clusterAddonConfig.AddonStages["BeforeClusterUpgrade"]))
-				for i, stage := range clusterAddonConfig.AddonStages["BeforeClusterUpgrade"] {
+				clusterAddon.Status.Stages = make([]csov1alpha1.StageStatus, len(clusterAddonConfig.AddonStages[beforeClusterUpgradeHook]))
+				for i, stage := range clusterAddonConfig.AddonStages[beforeClusterUpgradeHook] {
 					clusterAddon.Status.Stages[i].Name = stage.Name
 					clusterAddon.Status.Stages[i].Action = stage.Action
 					clusterAddon.Status.Stages[i].Phase = csov1alpha1.StagePhasePending
@@ -369,9 +374,9 @@ func (r *ClusterAddonReconciler) Reconcile(ctx context.Context, req reconcile.Re
 	// In case the Kubernetes version stayed the same during an upgrade, the hook server does not trigger and
 	// we just take the Helm charts that are supposed to be installed in the BeforeClusterUpgrade hook and apply them.
 	if oldRelease != nil && oldRelease.Meta.Versions.Kubernetes == releaseAsset.Meta.Versions.Kubernetes {
-		clusterAddon.Spec.Hook = "BeforeClusterUpgrade"
+		clusterAddon.Spec.Hook = beforeClusterUpgradeHook
 
-		for _, stage := range clusterAddonConfig.AddonStages["BeforeClusterUpgrade"] {
+		for _, stage := range clusterAddonConfig.AddonStages[beforeClusterUpgradeHook] {
 			shouldRequeue, err := r.executeStage(ctx, stage, in)
 			if err != nil {
 				return reconcile.Result{}, fmt.Errorf("failed to execute stage: %w", err)
@@ -445,8 +450,8 @@ func (r *ClusterAddonReconciler) Reconcile(ctx context.Context, req reconcile.Re
 		}
 	}
 
-	if clusterAddon.Spec.Hook == "AfterControlPlaneInitialized" || clusterAddon.Spec.Hook == "BeforeClusterUpgrade" {
-		if clusterAddon.Spec.Hook == "BeforeClusterUpgrade" {
+	if clusterAddon.Spec.Hook == afterControlPlaneInitialized || clusterAddon.Spec.Hook == beforeClusterUpgradeHook {
+		if clusterAddon.Spec.Hook == beforeClusterUpgradeHook {
 			// create the list of old release objects
 			oldClusterStackObjectList, err := r.getOldReleaseObjects(ctx, in, clusterAddonConfig, oldRelease)
 			if err != nil {
@@ -570,9 +575,9 @@ func (r *ClusterAddonReconciler) getOldReleaseObjects(ctx context.Context, in *t
 	)
 
 	if in.clusterAddon.HasStageAnnotation(csov1alpha1.StageAnnotationValueCreated) {
-		hook = "AfterControlPlaneInitialized"
+		hook = afterControlPlaneInitialized
 	} else {
-		hook = "BeforeClusterUpgrade"
+		hook = beforeClusterUpgradeHook
 	}
 
 	for _, stage := range clusterAddonConfig.AddonStages[hook] {
@@ -1332,7 +1337,7 @@ func clusterToClusterAddon(_ context.Context) handler.MapFunc {
 
 func unTarContent(src, dst string) error {
 	// Create the target directory if it doesn't exist
-	if err := os.MkdirAll(dst, os.ModePerm); err != nil {
+	if err := os.MkdirAll(dst, os.ModePerm); err != nil { //nolint:gosec // ignore permissions
 		return fmt.Errorf("%q: creating directory: %w", dst, err)
 	}
 
@@ -1366,12 +1371,12 @@ func unTarContent(src, dst string) error {
 		switch header.Typeflag {
 		case tar.TypeDir:
 			// Create directories
-			if err := os.MkdirAll(targetPath, os.ModePerm); err != nil {
+			if err := os.MkdirAll(targetPath, os.ModePerm); err != nil { //nolint:gosec // ignore permissions
 				return fmt.Errorf("%q: creating directory: %w", targetPath, err)
 			}
 		case tar.TypeReg:
 			// Create regular files
-			if err := os.MkdirAll(filepath.Dir(targetPath), os.ModePerm); err != nil {
+			if err := os.MkdirAll(filepath.Dir(targetPath), os.ModePerm); err != nil { //nolint:gosec // ignore permissions
 				return fmt.Errorf("%q: creating directory: %w", filepath.Dir(targetPath), err)
 			}
 

diff --git a/internal/controller/clusteraddon_controller_test.go b/internal/controller/clusteraddon_controller_test.go
@@ -61,7 +61,7 @@ var _ = Describe("ClusterAddonReconciler", func() {
 			},
 		}
 
-		testEnv.KubeClient.On("Apply", mock.Anything, mock.Anything, mock.Anything).Return([]*csov1alpha1.Resource{}, false, nil)
+		testEnv.KubeClient.On("Apply", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*csov1alpha1.Resource{}, false, nil)
 	})
 
 	AfterEach(func() {
@@ -240,7 +240,7 @@ var _ = Describe("ClusterAddonReconciler", func() {
 			}, timeout, interval).Should(BeTrue())
 
 			By("checking Update method was called")
-			Expect(testEnv.KubeClient.AssertCalled(GinkgoT(), "Apply", mock.Anything, mock.Anything, mock.Anything)).To(BeTrue())
+			Expect(testEnv.KubeClient.AssertCalled(GinkgoT(), "Apply", mock.Anything, mock.Anything, mock.Anything, mock.Anything)).To(BeTrue())
 		})
 
 		It("should not call update if the ClusterAddon version does not change in the ClusterClass update", func() {

diff --git a/internal/controller/clusteraddoncreate_controller_test.go b/internal/controller/clusteraddoncreate_controller_test.go
@@ -69,7 +69,7 @@ var _ = Describe("ClusterAddonCreateReconciler", func() {
 
 		key = types.NamespacedName{Name: fmt.Sprintf("cluster-addon-%s", cluster.Name), Namespace: testNs.Name}
 
-		testEnv.KubeClient.On("Apply", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*csov1alpha1.Resource{}, false, nil)
+		testEnv.KubeClient.On("Apply", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*csov1alpha1.Resource{}, false, nil)
 	})
 
 	AfterEach(func() {

diff --git a/internal/controller/clusterstackrelease_controller_test.go b/internal/controller/clusterstackrelease_controller_test.go
@@ -311,7 +311,7 @@ var _ = Describe("ClusterStackRelease validation", func() {
 				},
 			}
 
-			testEnv.KubeClient.On("Apply", mock.Anything, mock.Anything, mock.Anything).Return([]*csov1alpha1.Resource{}, false, nil)
+			testEnv.KubeClient.On("Apply", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*csov1alpha1.Resource{}, false, nil)
 		})
 
 		AfterEach(func() {

diff --git a/internal/test/integration/workloadcluster/controller_suite_test.go b/internal/test/integration/workloadcluster/controller_suite_test.go
@@ -31,7 +31,7 @@ import (
 )
 
 const (
-	timeout  = time.Second * 2
+	timeout  = time.Second * 3
 	interval = 100 * time.Millisecond
 
 	testClusterStackName   = "docker-ferrol-1-27-v1"

diff --git a/pkg/release/release.go b/pkg/release/release.go
@@ -104,6 +104,18 @@ func New(tag, downloadPath string) (Release, bool, error) {
 	return rel, false, nil
 }
 
+// ConvertFromClusterClassToClusterStackFormat converts `docker-ferrol-1-27-v0-sha.3960147` way to
+// `docker-ferrol-1-27-v0-sha-3960147`.
+func ConvertFromClusterClassToClusterStackFormat(input string) string {
+	parts := strings.Split(input, ".")
+
+	if len(parts) == 2 {
+		return fmt.Sprintf("%s-%s", parts[0], parts[1])
+	}
+
+	return input
+}
+
 func ensureMetadata(downloadPath, metadataFileName string) (Metadata, error) {
 	// Read the metadata.yaml file from the release.
 	metadataPath := filepath.Join(downloadPath, metadataFileName)
@@ -201,18 +213,6 @@ func (r *Release) Validate() error {
 	return nil
 }
 
-// ConvertFromClusterClassToClusterStackFormat converts `docker-ferrol-1-27-v0-sha.3960147` way to
-// `docker-ferrol-1-27-v0-sha-3960147`.
-func ConvertFromClusterClassToClusterStackFormat(input string) string {
-	parts := strings.Split(input, ".")
-
-	if len(parts) == 2 {
-		return fmt.Sprintf("%s-%s", parts[0], parts[1])
-	}
-
-	return input
-}
-
 // clusterAddonChartName returns the helm chart name for cluster addon.
 func (r *Release) clusterAddonChartName() string {
 	clusterAddonVersion, _ := version.ParseVersionString(r.Meta.Versions.Components.ClusterAddon)