Merge pull request #577 from giantswarm/update-main

Update from upstream main branch

.github/PULL_REQUEST_TEMPLATE.md

@@ -32,6 +32,7 @@ Fixes #
 
 - [ ] squashed commits
 - [ ] includes documentation
+- [ ] includes [emojis](https://github.com/kubernetes-sigs/kubebuilder-release-tools?tab=readme-ov-file#kubebuilder-project-versioning)
 - [ ] adds unit tests
 - [ ] adds or updates e2e tests
 

.github/dependabot.yml

@@ -3,25 +3,85 @@ updates:
   - package-ecosystem: "gomod"
     directory: "/"
     schedule:
-      interval: "daily"
+      interval: "weekly"
+    commit-message:
+      prefix: ":seedling:"
+    labels:
+      - "kind/cleanup"
+    groups:
+      dependencies:
+        patterns:
+          - "*"
+    ignore:
+      # Ignore Cluster-API as its upgraded manually.
+      - dependency-name: "sigs.k8s.io/cluster-api*"
+      # Ignore controller-runtime as its upgraded manually.
+      - dependency-name: "sigs.k8s.io/controller-runtime"
+      # Ignore k8s and its transitives modules as they are upgraded manually
+      # together with controller-runtime.
+      - dependency-name: "k8s.io/*"
+      - dependency-name: "go.etcd.io/*"
+      - dependency-name: "google.golang.org/grpc"
 
   - package-ecosystem: "docker"
     directory: "/"
     schedule:
       interval: "weekly"
+    commit-message:
+      prefix: ":seedling:"
+    labels:
+      - "kind/cleanup"
+    groups:
+      dependencies:
+        patterns:
+          - "*"
 
   # Enable version updates for Go tools
   - package-ecosystem: "gomod"
     directory: "/hack/tools"
     schedule:
       interval: "weekly"
+    commit-message:
+      prefix: ":seedling:"
+    labels:
+      - "kind/cleanup"
+    groups:
+      dependencies:
+        patterns:
+          - "*"
+    ignore:
+      # Ignore Cluster-API as its upgraded manually.
+      - dependency-name: "sigs.k8s.io/cluster-api*"
+      # Ignore controller-runtime as its upgraded manually.
+      - dependency-name: "sigs.k8s.io/controller-runtime"
+      # Ignore k8s and its transitives modules as they are upgraded manually
+      # together with controller-runtime.
+      - dependency-name: "k8s.io/*"
+      # Ignore controller-tools as its upgraded manually.
+      - dependency-name: "sigs.k8s.io/controller-tools"
 
   - package-ecosystem: "docker"
     directory: "/hack/tools"
     schedule:
       interval: "weekly"
+    commit-message:
+      prefix: ":seedling:"
+    labels:
+      - "kind/cleanup"
+    groups:
+      dependencies:
+        patterns:
+          - "*"
 
   - package-ecosystem: "github-actions"
     directory: "/"
     schedule:
-      interval: "daily"
+      interval: "weekly"
+    commit-message:
+      prefix: ":seedling:"
+    labels:
+      - "kind/cleanup"
+    groups:
+      dependencies:
+        patterns:
+          - "*"

.github/workflows/codeql-analysis.yml

@@ -25,14 +25,14 @@ jobs:
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@v2
+      uses: github/codeql-action/init@v3
       with:
         languages: ${{ matrix.language }}
 
     # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
     # If this step fails, then you should remove it and run the build manually (see below)
     - name: Autobuild
-      uses: github/codeql-action/autobuild@v2
+      uses: github/codeql-action/autobuild@v3
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@v2
+      uses: github/codeql-action/analyze@v3

.github/workflows/dependabot.yml

@@ -19,7 +19,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - name: Set up Go 1.x
-      uses: actions/setup-go@v4
+      uses: actions/setup-go@v5
       with:
         go-version: '1.20'
       id: go

.github/workflows/pr-gh-workflow-approve.yaml

@@ -17,7 +17,7 @@ jobs:
       actions: write
     steps:
       - name: Update PR
-        uses: actions/github-script@e69ef5462fd455e02edcaf4dd7708eda96b9eda0 # v7.0.0
+        uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
         continue-on-error: true
         with:
           github-token: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/pr-verify.yml

@@ -0,0 +1,16 @@
+name: PR verify
+
+on:
+  pull_request_target:
+    types: [opened, edited, synchronize, reopened]
+
+jobs:
+  verify:
+    runs-on: ubuntu-latest
+    name: verify PR contents
+    steps:
+    - name: Verifier action
+      id: verifier
+      uses: kubernetes-sigs/kubebuilder-release-tools@012269a88fa4c034a0acf1ba84c26b195c0dbab4 # tag=v0.4.3
+      with:
+        github_token: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/scan.yml

@@ -22,7 +22,7 @@ jobs:
         with:
           ref: ${{ matrix.branch }}
       - name: Setup go
-        uses: actions/setup-go@v4
+        uses: actions/setup-go@v5
         with:
           go-version-file: '${{ github.workspace }}/go.mod'
       - name: Run verify container script

OWNERS_ALIASES

@@ -30,3 +30,4 @@ aliases:
     - nrb
     - faiq
     - fiunchinho
+    - AndiDog

exp/api/v1beta2/awsmachinepool_webhook.go

@@ -108,6 +108,13 @@ func (r *AWSMachinePool) validateAdditionalSecurityGroups() field.ErrorList {
 	}
 	return allErrs
 }
+func (r *AWSMachinePool) validateSpotInstances() field.ErrorList {
+	var allErrs field.ErrorList
+	if r.Spec.AWSLaunchTemplate.SpotMarketOptions != nil && r.Spec.MixedInstancesPolicy != nil {
+		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec.awsLaunchTemplate.spotMarketOptions"), "either spec.awsLaunchTemplate.spotMarketOptions or spec.mixedInstancesPolicy should be used"))
+	}
+	return allErrs
+}
 
 // ValidateCreate will do any extra validation when creating a AWSMachinePool.
 func (r *AWSMachinePool) ValidateCreate() (admission.Warnings, error) {
@@ -120,6 +127,7 @@ func (r *AWSMachinePool) ValidateCreate() (admission.Warnings, error) {
 	allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
 	allErrs = append(allErrs, r.validateSubnets()...)
 	allErrs = append(allErrs, r.validateAdditionalSecurityGroups()...)
+	allErrs = append(allErrs, r.validateSpotInstances()...)
 
 	if len(allErrs) == 0 {
 		return nil, nil
@@ -140,6 +148,7 @@ func (r *AWSMachinePool) ValidateUpdate(old runtime.Object) (admission.Warnings,
 	allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
 	allErrs = append(allErrs, r.validateSubnets()...)
 	allErrs = append(allErrs, r.validateAdditionalSecurityGroups()...)
+	allErrs = append(allErrs, r.validateSpotInstances()...)
 
 	if len(allErrs) == 0 {
 		return nil, nil

exp/api/v1beta2/awsmachinepool_webhook_test.go

@@ -139,6 +139,20 @@ func TestAWSMachinePoolValidateCreate(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "Should fail if both spot market options or mixed instances policy are set",
+			pool: &AWSMachinePool{
+				Spec: AWSMachinePoolSpec{
+					MixedInstancesPolicy: &MixedInstancesPolicy{
+						Overrides: []Overrides{{InstanceType: "t3.medium"}},
+					},
+					AWSLaunchTemplate: AWSLaunchTemplate{
+						SpotMarketOptions: &infrav1.SpotMarketOptions{MaxPrice: aws.String("0.1")},
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -252,6 +266,27 @@ func TestAWSMachinePoolValidateUpdate(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "Should fail update if both spec.awsLaunchTemplate.SpotMarketOptions and spec.MixedInstancesPolicy are passed in AWSMachinePool spec",
+			old: &AWSMachinePool{
+				Spec: AWSMachinePoolSpec{
+					MixedInstancesPolicy: &MixedInstancesPolicy{
+						Overrides: []Overrides{{InstanceType: "t3.medium"}},
+					},
+				},
+			},
+			new: &AWSMachinePool{
+				Spec: AWSMachinePoolSpec{
+					MixedInstancesPolicy: &MixedInstancesPolicy{
+						Overrides: []Overrides{{InstanceType: "t3.medium"}},
+					},
+					AWSLaunchTemplate: AWSLaunchTemplate{
+						SpotMarketOptions: &infrav1.SpotMarketOptions{MaxPrice: pointer.String("0.1")},
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

exp/controllers/awsmachinepool_controller.go

@@ -49,6 +49,7 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	expclusterv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
+	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/cluster-api/util/predicates"
 )
@@ -131,6 +132,7 @@ func (r *AWSMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	// Create the machine pool scope
 	machinePoolScope, err := scope.NewMachinePoolScope(scope.MachinePoolScopeParams{
 		Client:         r.Client,
+		Logger:         log,
 		Cluster:        cluster,
 		MachinePool:    machinePool,
 		InfraCluster:   infraCluster,
@@ -226,13 +228,30 @@ func (r *AWSMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP
 	ec2Svc := r.getEC2Service(ec2Scope)
 	asgsvc := r.getASGService(clusterScope)
 
+	// Find existing ASG
+	asg, err := r.findASG(machinePoolScope, asgsvc)
+	if err != nil {
+		conditions.MarkUnknown(machinePoolScope.AWSMachinePool, expinfrav1.ASGReadyCondition, expinfrav1.ASGNotFoundReason, err.Error())
+		return err
+	}
+
 	canUpdateLaunchTemplate := func() (bool, error) {
 		// If there is a change: before changing the template, check if there exist an ongoing instance refresh,
 		// because only 1 instance refresh can be "InProgress". If template is updated when refresh cannot be started,
 		// that change will not trigger a refresh. Do not start an instance refresh if only userdata changed.
+		if asg == nil {
+			// If the ASG hasn't been created yet, there is no need to check if we can start the instance refresh.
+			// But we want to update the LaunchTemplate because an error in the LaunchTemplate may be blocking the ASG creation.
+			return true, nil
+		}
 		return asgsvc.CanStartASGInstanceRefresh(machinePoolScope)
 	}
 	runPostLaunchTemplateUpdateOperation := func() error {
+		// skip instance refresh if ASG is not created yet
+		if asg == nil {
+			machinePoolScope.Debug("ASG does not exist yet, skipping instance refresh")
+			return nil
+		}
 		// skip instance refresh if explicitly disabled
 		if machinePoolScope.AWSMachinePool.Spec.RefreshPreferences != nil && machinePoolScope.AWSMachinePool.Spec.RefreshPreferences.Disable {
 			machinePoolScope.Debug("instance refresh disabled, skipping instance refresh")
@@ -259,13 +278,6 @@ func (r *AWSMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP
 	// set the LaunchTemplateReady condition
 	conditions.MarkTrue(machinePoolScope.AWSMachinePool, expinfrav1.LaunchTemplateReadyCondition)
 
-	// Find existing ASG
-	asg, err := r.findASG(machinePoolScope, asgsvc)
-	if err != nil {
-		conditions.MarkUnknown(machinePoolScope.AWSMachinePool, expinfrav1.ASGReadyCondition, expinfrav1.ASGNotFoundReason, err.Error())
-		return err
-	}
-
 	if asg == nil {
 		// Create new ASG
 		if err := r.createPool(machinePoolScope, clusterScope); err != nil {
@@ -275,7 +287,7 @@ func (r *AWSMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP
 		return nil
 	}
 
-	if scope.ReplicasExternallyManaged(machinePoolScope.MachinePool) {
+	if annotations.ReplicasManagedByExternalAutoscaler(machinePoolScope.MachinePool) {
 		// Set MachinePool replicas to the ASG DesiredCapacity
 		if *machinePoolScope.MachinePool.Spec.Replicas != *asg.DesiredCapacity {
 			machinePoolScope.Info("Setting MachinePool replicas to ASG DesiredCapacity",
@@ -503,7 +515,7 @@ func (r *AWSMachinePoolReconciler) findASG(machinePoolScope *scope.MachinePoolSc
 func diffASG(machinePoolScope *scope.MachinePoolScope, existingASG *expinfrav1.AutoScalingGroup) string {
 	detectedMachinePoolSpec := machinePoolScope.MachinePool.Spec.DeepCopy()
 
-	if !scope.ReplicasExternallyManaged(machinePoolScope.MachinePool) {
+	if !annotations.ReplicasManagedByExternalAutoscaler(machinePoolScope.MachinePool) {
 		detectedMachinePoolSpec.Replicas = existingASG.DesiredCapacity
 	}
 	if diff := cmp.Diff(machinePoolScope.MachinePool.Spec, *detectedMachinePoolSpec); diff != "" {

exp/controllers/awsmachinepool_controller_test.go

@@ -208,8 +208,6 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
 				defer teardown(t, g)
 				getASG(t, g)
 
-				ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any())
-
 				_ = reconciler.reconcileNormal(context.Background(), ms, cs, cs)
 
 				g.Expect(ms.AWSMachinePool.Finalizers).To(ContainElement(expinfrav1.MachinePoolFinalizer))
@@ -253,11 +251,18 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
 				t.Helper()
 				ms.AWSMachinePool.Spec.ProviderID = id
 			}
+			getASG := func(t *testing.T, g *WithT) {
+				t.Helper()
+
+				ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil).AnyTimes()
+				asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil).AnyTimes()
+			}
 			t.Run("should look up by provider ID when one exists", func(t *testing.T) {
 				g := NewWithT(t)
 				setup(t, g)
 				defer teardown(t, g)
 				setProviderID(t, g)
+				getASG(t, g)
 
 				expectedErr := errors.New("no connection available ")
 				ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(expectedErr)
@@ -380,7 +385,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
 			ec2Svc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
 
 			ms.MachinePool.Annotations = map[string]string{
-				scope.ReplicasManagedByAnnotation: scope.ExternalAutoscalerReplicasManagedByAnnotationValue,
+				clusterv1.ReplicasManagedByAnnotation: "somehow-externally-managed",
 			}
 			ms.MachinePool.Spec.Replicas = pointer.Int32(0)
 
@@ -908,7 +913,7 @@ func TestDiffASG(t *testing.T) {
 					MachinePool: &expclusterv1.MachinePool{
 						ObjectMeta: metav1.ObjectMeta{
 							Annotations: map[string]string{
-								scope.ReplicasManagedByAnnotation: scope.ExternalAutoscalerReplicasManagedByAnnotationValue,
+								clusterv1.ReplicasManagedByAnnotation: "", // empty value counts as true (= externally managed)
 							},
 						},
 						Spec: expclusterv1.MachinePoolSpec{