Fix review comments 4

Author: LiangquanLi930

api/v1beta1/awsmachine_conversion.go

@@ -138,6 +138,7 @@ func (r *AWSMachineTemplate) ConvertTo(dstRaw conversion.Hub) error {
 
 	// Restore Status fields that don't exist in v1beta1.
 	dst.Status.NodeInfo = restored.Status.NodeInfo
+	dst.Status.Conditions = restored.Status.Conditions
 
 	return nil
 }

api/v1beta1/conversion.go

@@ -110,6 +110,6 @@ func Convert_v1beta2_AWSMachineStatus_To_v1beta1_AWSMachineStatus(in *v1beta2.AW
 }
 
 func Convert_v1beta2_AWSMachineTemplateStatus_To_v1beta1_AWSMachineTemplateStatus(in *v1beta2.AWSMachineTemplateStatus, out *AWSMachineTemplateStatus, s conversion.Scope) error {
-	// NodeInfo field is ignored (dropped) as it doesn't exist in v1beta1
+	// NodeInfo and Conditions fields are ignored (dropped) as they don't exist in v1beta1
 	return autoConvert_v1beta2_AWSMachineTemplateStatus_To_v1beta1_AWSMachineTemplateStatus(in, out, s)
 }

api/v1beta1/zz_generated.conversion.go

@@ -72,6 +72,10 @@ type AWSMachineTemplateStatus struct {
 	// https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20210310-opt-in-autoscaling-from-zero.md
 	// +optional
 	NodeInfo *NodeInfo `json:"nodeInfo,omitempty"`
+
+	// Conditions defines current service state of the AWSMachineTemplate.
+	// +optional
+	Conditions clusterv1.Conditions `json:"conditions,omitempty"`
 }
 
 // AWSMachineTemplateSpec defines the desired state of AWSMachineTemplate.
@@ -114,6 +118,16 @@ type AWSMachineTemplateResource struct {
 	Spec AWSMachineSpec `json:"spec"`
 }
 
+// GetConditions returns the observations of the operational state of the AWSMachineTemplate resource.
+func (r *AWSMachineTemplate) GetConditions() clusterv1.Conditions {
+	return r.Status.Conditions
+}
+
+// SetConditions sets the underlying service state of the AWSMachineTemplate to the predescribed clusterv1.Conditions.
+func (r *AWSMachineTemplate) SetConditions(conditions clusterv1.Conditions) {
+	r.Status.Conditions = conditions
+}
+
 func init() {
 	SchemeBuilder.Register(&AWSMachineTemplate{}, &AWSMachineTemplateList{})
 }

api/v1beta2/awsmachinetemplate_types.go

@@ -1156,6 +1156,58 @@ spec:
                   This value is used for autoscaling from zero operations as defined in:
                   https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20210310-opt-in-autoscaling-from-zero.md
                 type: object
+              conditions:
+                description: Conditions defines current service state of the AWSMachineTemplate.
+                items:
+                  description: Condition defines an observation of a Cluster API resource
+                    operational state.
+                  properties:
+                    lastTransitionTime:
+                      description: |-
+                        lastTransitionTime is the last time the condition transitioned from one status to another.
+                        This should be when the underlying condition changed. If that is not known, then using the time when
+                        the API field changed is acceptable.
+                      format: date-time
+                      type: string
+                    message:
+                      description: |-
+                        message is a human readable message indicating details about the transition.
+                        This field may be empty.
+                      maxLength: 10240
+                      minLength: 1
+                      type: string
+                    reason:
+                      description: |-
+                        reason is the reason for the condition's last transition in CamelCase.
+                        The specific API may choose whether or not this field is considered a guaranteed API.
+                        This field may be empty.
+                      maxLength: 256
+                      minLength: 1
+                      type: string
+                    severity:
+                      description: |-
+                        severity provides an explicit classification of Reason code, so the users or machines can immediately
+                        understand the current situation and act accordingly.
+                        The Severity field MUST be set only when Status=False.
+                      maxLength: 32
+                      type: string
+                    status:
+                      description: status of the condition, one of True, False, Unknown.
+                      type: string
+                    type:
+                      description: |-
+                        type of condition in CamelCase or in foo.example.com/CamelCase.
+                        Many .condition.type values are consistent across resources like Available, but because arbitrary conditions
+                        can be useful (see .node.status.conditions), the ability to deconflict is important.
+                      maxLength: 256
+                      minLength: 1
+                      type: string
+                  required:
+                  - lastTransitionTime
+                  - status
+                  - type
+                  type: object
+                type: array
               nodeInfo:
                 description: |-
                   NodeInfo contains information about the node's architecture and operating system.

api/v1beta2/zz_generated.deepcopy.go

@@ -40,6 +40,7 @@ import (
 	ec2service "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/ec2"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/record"
+	"sigs.k8s.io/cluster-api-provider-aws/v2/util/paused"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
@@ -101,8 +102,22 @@ func (r *AWSMachineTemplateReconciler) Reconcile(ctx context.Context, req ctrl.R
 		return ctrl.Result{}, nil
 	}
 
+	// Get the owner cluster
+	cluster, err := util.GetOwnerCluster(ctx, r.Client, awsMachineTemplate.ObjectMeta)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+	if cluster == nil {
+		return ctrl.Result{}, nil
+	}
+
+	// Check if the resource is paused
+	if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, r.Client, cluster, awsMachineTemplate); err != nil || isPaused || conditionChanged {
+		return ctrl.Result{}, err
+	}
+
 	// Find the region by checking ownerReferences
-	region, err := r.getRegion(ctx, awsMachineTemplate)
+	region, err := r.getRegion(ctx, cluster)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -155,12 +170,7 @@ func (r *AWSMachineTemplateReconciler) Reconcile(ctx context.Context, req ctrl.R
 }
 
 // getRegion finds the region by checking the template's owner cluster reference.
-func (r *AWSMachineTemplateReconciler) getRegion(ctx context.Context, template *infrav1.AWSMachineTemplate) (string, error) {
-	// Get the owner cluster
-	cluster, err := util.GetOwnerCluster(ctx, r.Client, template.ObjectMeta)
-	if err != nil {
-		return "", err
-	}
+func (r *AWSMachineTemplateReconciler) getRegion(ctx context.Context, cluster *clusterv1.Cluster) (string, error) {
 	if cluster == nil {
 		return "", errors.New("no owner cluster found")
 	}

config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml

@@ -69,14 +69,6 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
 	t.Run("getRegion", func(t *testing.T) {
 		t.Run("should get region from AWSCluster", func(t *testing.T) {
 			g := NewWithT(t)
-			template := newAWSMachineTemplate("test-template")
-			template.OwnerReferences = []metav1.OwnerReference{
-				{
-					APIVersion: clusterv1.GroupVersion.String(),
-					Kind:       "Cluster",
-					Name:       "test-cluster",
-				},
-			}
 			cluster := &clusterv1.Cluster{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "test-cluster",
@@ -101,24 +93,23 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
 			}
 
 			reconciler := &AWSMachineTemplateReconciler{
-				Client: newFakeClient(template, cluster, awsCluster),
+				Client: newFakeClient(cluster, awsCluster),
 			}
 
-			region, err := reconciler.getRegion(context.Background(), template)
+			region, err := reconciler.getRegion(context.Background(), cluster)
 
 			g.Expect(err).To(BeNil())
 			g.Expect(region).To(Equal("us-west-2"))
 		})
 
-		t.Run("should return error when no owner cluster found", func(t *testing.T) {
+		t.Run("should return error when cluster is nil", func(t *testing.T) {
 			g := NewWithT(t)
-			template := newAWSMachineTemplate("test-template")
 
 			reconciler := &AWSMachineTemplateReconciler{
-				Client: newFakeClient(template),
+				Client: newFakeClient(),
 			}
 
-			region, err := reconciler.getRegion(context.Background(), template)
+			region, err := reconciler.getRegion(context.Background(), nil)
 
 			g.Expect(err).ToNot(BeNil())
 			g.Expect(err.Error()).To(ContainSubstring("no owner cluster found"))
@@ -127,14 +118,6 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
 
 		t.Run("should return empty when cluster has no infrastructure ref", func(t *testing.T) {
 			g := NewWithT(t)
-			template := newAWSMachineTemplate("test-template")
-			template.OwnerReferences = []metav1.OwnerReference{
-				{
-					APIVersion: clusterv1.GroupVersion.String(),
-					Kind:       "Cluster",
-					Name:       "test-cluster",
-				},
-			}
 			cluster := &clusterv1.Cluster{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "test-cluster",
@@ -143,25 +126,17 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
 			}
 
 			reconciler := &AWSMachineTemplateReconciler{
-				Client: newFakeClient(template, cluster),
+				Client: newFakeClient(cluster),
 			}
 
-			region, err := reconciler.getRegion(context.Background(), template)
+			region, err := reconciler.getRegion(context.Background(), cluster)
 
 			g.Expect(err).To(BeNil())
 			g.Expect(region).To(Equal(""))
 		})
 
 		t.Run("should return empty when AWSCluster not found", func(t *testing.T) {
 			g := NewWithT(t)
-			template := newAWSMachineTemplate("test-template")
-			template.OwnerReferences = []metav1.OwnerReference{
-				{
-					APIVersion: clusterv1.GroupVersion.String(),
-					Kind:       "Cluster",
-					Name:       "test-cluster",
-				},
-			}
 			cluster := &clusterv1.Cluster{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "test-cluster",
@@ -177,10 +152,10 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
 			}
 
 			reconciler := &AWSMachineTemplateReconciler{
-				Client: newFakeClient(template, cluster),
+				Client: newFakeClient(cluster),
 			}
 
-			region, err := reconciler.getRegion(context.Background(), template)
+			region, err := reconciler.getRegion(context.Background(), cluster)
 
 			g.Expect(err).To(BeNil())
 			g.Expect(region).To(Equal(""))
@@ -284,7 +259,99 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
 			g.Expect(result.Requeue).To(BeFalse())
 		})
 
-		t.Run("should return error when no owner cluster", func(t *testing.T) {
+		t.Run("should not reconcile when cluster is paused", func(t *testing.T) {
+			g := NewWithT(t)
+			template := newAWSMachineTemplate("test-template")
+			template.OwnerReferences = []metav1.OwnerReference{
+				{
+					APIVersion: clusterv1.GroupVersion.String(),
+					Kind:       "Cluster",
+					Name:       "test-cluster",
+				},
+			}
+			cluster := &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-cluster",
+					Namespace: "default",
+				},
+				Spec: clusterv1.ClusterSpec{
+					Paused: true,
+					InfrastructureRef: &corev1.ObjectReference{
+						Kind:      "AWSCluster",
+						Name:      "test-aws-cluster",
+						Namespace: "default",
+					},
+				},
+			}
+			awsCluster := &infrav1.AWSCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-aws-cluster",
+					Namespace: "default",
+				},
+				Spec: infrav1.AWSClusterSpec{
+					Region: "us-west-2",
+				},
+			}
+
+			reconciler := &AWSMachineTemplateReconciler{
+				Client: newFakeClient(template, cluster, awsCluster),
+			}
+
+			result, err := reconciler.Reconcile(context.Background(), ctrl.Request{
+				NamespacedName: client.ObjectKeyFromObject(template),
+			})
+
+			g.Expect(err).To(BeNil())
+			g.Expect(result.Requeue).To(BeFalse())
+		})
+
+		t.Run("should not reconcile when template has paused annotation", func(t *testing.T) {
+			g := NewWithT(t)
+			template := newAWSMachineTemplate("test-template")
+			template.Annotations = map[string]string{clusterv1.PausedAnnotation: ""}
+			template.OwnerReferences = []metav1.OwnerReference{
+				{
+					APIVersion: clusterv1.GroupVersion.String(),
+					Kind:       "Cluster",
+					Name:       "test-cluster",
+				},
+			}
+			cluster := &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-cluster",
+					Namespace: "default",
+				},
+				Spec: clusterv1.ClusterSpec{
+					InfrastructureRef: &corev1.ObjectReference{
+						Kind:      "AWSCluster",
+						Name:      "test-aws-cluster",
+						Namespace: "default",
+					},
+				},
+			}
+			awsCluster := &infrav1.AWSCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-aws-cluster",
+					Namespace: "default",
+				},
+				Spec: infrav1.AWSClusterSpec{
+					Region: "us-west-2",
+				},
+			}
+
+			reconciler := &AWSMachineTemplateReconciler{
+				Client: newFakeClient(template, cluster, awsCluster),
+			}
+
+			result, err := reconciler.Reconcile(context.Background(), ctrl.Request{
+				NamespacedName: client.ObjectKeyFromObject(template),
+			})
+
+			g.Expect(err).To(BeNil())
+			g.Expect(result.Requeue).To(BeFalse())
+		})
+
+		t.Run("should skip when no owner cluster", func(t *testing.T) {
 			g := NewWithT(t)
 			template := newAWSMachineTemplate("test-template")
 
@@ -296,8 +363,7 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
 				NamespacedName: client.ObjectKeyFromObject(template),
 			})
 
-			g.Expect(err).ToNot(BeNil())
-			g.Expect(err.Error()).To(ContainSubstring("no owner cluster found"))
+			g.Expect(err).To(BeNil())
 			g.Expect(result.Requeue).To(BeFalse())
 		})