🌱 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]>

Author: janiskemper

.github/workflows/build.yml

@@ -4,7 +4,6 @@ on: # yamllint disable-line rule:truthy
   push:
     branches:
       - main
-      - addons
   # If the cache was cleaned we should re-build the cache with the latest commit
   workflow_run:
     workflows:

.github/workflows/kubebuilder-markers-checker.yml

@@ -4,11 +4,9 @@ on:
   push:
     branches:
       - main
-      - addons
   pull_request:
     branches:
       - main
-      - addons
     paths:
       - "api/**"
       - ".github/workflows/kubebuilder-markers-checker.yml"

.github/workflows/pr-lint.yml

@@ -4,7 +4,6 @@ on: # yamllint disable-line rule:truthy
     types: [opened, synchronize, reopened, ready_for_review]
     branches:
       - main
-      - addons
       - "releases/**"
     paths:
       - "!**/vendor/**"

.github/workflows/test.yml

@@ -6,7 +6,6 @@ on: # yamllint disable-line rule:truthy
     types: [opened, synchronize, reopened, ready_for_review]
     branches:
       - main
-      - addons
       - "releases/**"
     paths:
       - "**.go"

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"])
 

capd.yaml

@@ -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)
 	}
 

cmd/main.go

@@ -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

config/default/kustomization.yaml

@@ -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)
 			}
 

internal/controller/clusteraddon_controller.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() {