Skip to content

Commit 641a0ca

Browse files
LiangquanLi930LiangquanLi930
committed
Fix review comments 4
1 parent c0657c7 commit 641a0ca

8 files changed

+102
-42
lines changed

‎api/v1beta1/awsmachine_conversion.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ func (r *AWSMachineTemplate) ConvertTo(dstRaw conversion.Hub) error {
138138

139139
// Restore Status fields that don't exist in v1beta1.
140140
dst.Status.NodeInfo = restored.Status.NodeInfo
141+
dst.Status.Conditions = restored.Status.Conditions
141142

142143
return nil
143144
}

‎api/v1beta1/conversion.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,6 @@ func Convert_v1beta2_AWSMachineStatus_To_v1beta1_AWSMachineStatus(in *v1beta2.AW
110110
}
111111

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

‎api/v1beta1/zz_generated.conversion.go‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1beta2/awsmachinetemplate_types.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ type AWSMachineTemplateStatus struct {
7272
// https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20210310-opt-in-autoscaling-from-zero.md
7373
// +optional
7474
NodeInfo *NodeInfo `json:"nodeInfo,omitempty"`
75+
76+
// Conditions defines current service state of the AWSMachineTemplate.
77+
// +optional
78+
Conditions clusterv1.Conditions `json:"conditions,omitempty"`
7579
}
7680

7781
// AWSMachineTemplateSpec defines the desired state of AWSMachineTemplate.
@@ -114,6 +118,16 @@ type AWSMachineTemplateResource struct {
114118
Spec AWSMachineSpec `json:"spec"`
115119
}
116120

121+
// GetConditions returns the observations of the operational state of the AWSMachineTemplate resource.
122+
func (r *AWSMachineTemplate) GetConditions() clusterv1.Conditions {
123+
return r.Status.Conditions
124+
}
125+
126+
// SetConditions sets the underlying service state of the AWSMachineTemplate to the predescribed clusterv1.Conditions.
127+
func (r *AWSMachineTemplate) SetConditions(conditions clusterv1.Conditions) {
128+
r.Status.Conditions = conditions
129+
}
130+
117131
func init() {
118132
SchemeBuilder.Register(&AWSMachineTemplate{}, &AWSMachineTemplateList{})
119133
}

‎api/v1beta2/zz_generated.deepcopy.go‎

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,58 @@ spec:
11561156
This value is used for autoscaling from zero operations as defined in:
11571157
https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20210310-opt-in-autoscaling-from-zero.md
11581158
type: object
1159+
conditions:
1160+
description: Conditions defines current service state of the AWSMachineTemplate.
1161+
items:
1162+
description: Condition defines an observation of a Cluster API resource
1163+
operational state.
1164+
properties:
1165+
lastTransitionTime:
1166+
description: |-
1167+
lastTransitionTime is the last time the condition transitioned from one status to another.
1168+
This should be when the underlying condition changed. If that is not known, then using the time when
1169+
the API field changed is acceptable.
1170+
format: date-time
1171+
type: string
1172+
message:
1173+
description: |-
1174+
message is a human readable message indicating details about the transition.
1175+
This field may be empty.
1176+
maxLength: 10240
1177+
minLength: 1
1178+
type: string
1179+
reason:
1180+
description: |-
1181+
reason is the reason for the condition's last transition in CamelCase.
1182+
The specific API may choose whether or not this field is considered a guaranteed API.
1183+
This field may be empty.
1184+
maxLength: 256
1185+
minLength: 1
1186+
type: string
1187+
severity:
1188+
description: |-
1189+
severity provides an explicit classification of Reason code, so the users or machines can immediately
1190+
understand the current situation and act accordingly.
1191+
The Severity field MUST be set only when Status=False.
1192+
maxLength: 32
1193+
type: string
1194+
status:
1195+
description: status of the condition, one of True, False, Unknown.
1196+
type: string
1197+
type:
1198+
description: |-
1199+
type of condition in CamelCase or in foo.example.com/CamelCase.
1200+
Many .condition.type values are consistent across resources like Available, but because arbitrary conditions
1201+
can be useful (see .node.status.conditions), the ability to deconflict is important.
1202+
maxLength: 256
1203+
minLength: 1
1204+
type: string
1205+
required:
1206+
- lastTransitionTime
1207+
- status
1208+
- type
1209+
type: object
1210+
type: array
11591211
nodeInfo:
11601212
description: |-
11611213
NodeInfo contains information about the node's architecture and operating system.

‎controllers/awsmachinetemplate_controller.go‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
ec2service "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/ec2"
4141
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
4242
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/record"
43+
"sigs.k8s.io/cluster-api-provider-aws/v2/util/paused"
4344
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
4445
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
4546
"sigs.k8s.io/cluster-api/util"
@@ -101,8 +102,22 @@ func (r *AWSMachineTemplateReconciler) Reconcile(ctx context.Context, req ctrl.R
101102
return ctrl.Result{}, nil
102103
}
103104

105+
// Get the owner cluster
106+
cluster, err := util.GetOwnerCluster(ctx, r.Client, awsMachineTemplate.ObjectMeta)
107+
if err != nil {
108+
return ctrl.Result{}, err
109+
}
110+
if cluster == nil {
111+
return ctrl.Result{}, nil
112+
}
113+
114+
// Check if the resource is paused
115+
if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, r.Client, cluster, awsMachineTemplate); err != nil || isPaused || conditionChanged {
116+
return ctrl.Result{}, err
117+
}
118+
104119
// Find the region by checking ownerReferences
105-
region, err := r.getRegion(ctx, awsMachineTemplate)
120+
region, err := r.getRegion(ctx, cluster)
106121
if err != nil {
107122
return ctrl.Result{}, err
108123
}
@@ -155,12 +170,7 @@ func (r *AWSMachineTemplateReconciler) Reconcile(ctx context.Context, req ctrl.R
155170
}
156171

157172
// getRegion finds the region by checking the template's owner cluster reference.
158-
func (r *AWSMachineTemplateReconciler) getRegion(ctx context.Context, template *infrav1.AWSMachineTemplate) (string, error) {
159-
// Get the owner cluster
160-
cluster, err := util.GetOwnerCluster(ctx, r.Client, template.ObjectMeta)
161-
if err != nil {
162-
return "", err
163-
}
173+
func (r *AWSMachineTemplateReconciler) getRegion(ctx context.Context, cluster *clusterv1.Cluster) (string, error) {
164174
if cluster == nil {
165175
return "", errors.New("no owner cluster found")
166176
}

‎controllers/awsmachinetemplate_controller_unit_test.go‎

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,6 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
6969
t.Run("getRegion", func(t *testing.T) {
7070
t.Run("should get region from AWSCluster", func(t *testing.T) {
7171
g := NewWithT(t)
72-
template := newAWSMachineTemplate("test-template")
73-
template.OwnerReferences = []metav1.OwnerReference{
74-
{
75-
APIVersion: clusterv1.GroupVersion.String(),
76-
Kind: "Cluster",
77-
Name: "test-cluster",
78-
},
79-
}
8072
cluster := &clusterv1.Cluster{
8173
ObjectMeta: metav1.ObjectMeta{
8274
Name: "test-cluster",
@@ -101,24 +93,23 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
10193
}
10294

10395
reconciler := &AWSMachineTemplateReconciler{
104-
Client: newFakeClient(template, cluster, awsCluster),
96+
Client: newFakeClient(cluster, awsCluster),
10597
}
10698

107-
region, err := reconciler.getRegion(context.Background(), template)
99+
region, err := reconciler.getRegion(context.Background(), cluster)
108100

109101
g.Expect(err).To(BeNil())
110102
g.Expect(region).To(Equal("us-west-2"))
111103
})
112104

113-
t.Run("should return error when no owner cluster found", func(t *testing.T) {
105+
t.Run("should return error when cluster is nil", func(t *testing.T) {
114106
g := NewWithT(t)
115-
template := newAWSMachineTemplate("test-template")
116107

117108
reconciler := &AWSMachineTemplateReconciler{
118-
Client: newFakeClient(template),
109+
Client: newFakeClient(),
119110
}
120111

121-
region, err := reconciler.getRegion(context.Background(), template)
112+
region, err := reconciler.getRegion(context.Background(), nil)
122113

123114
g.Expect(err).ToNot(BeNil())
124115
g.Expect(err.Error()).To(ContainSubstring("no owner cluster found"))
@@ -127,14 +118,6 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
127118

128119
t.Run("should return empty when cluster has no infrastructure ref", func(t *testing.T) {
129120
g := NewWithT(t)
130-
template := newAWSMachineTemplate("test-template")
131-
template.OwnerReferences = []metav1.OwnerReference{
132-
{
133-
APIVersion: clusterv1.GroupVersion.String(),
134-
Kind: "Cluster",
135-
Name: "test-cluster",
136-
},
137-
}
138121
cluster := &clusterv1.Cluster{
139122
ObjectMeta: metav1.ObjectMeta{
140123
Name: "test-cluster",
@@ -143,25 +126,17 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
143126
}
144127

145128
reconciler := &AWSMachineTemplateReconciler{
146-
Client: newFakeClient(template, cluster),
129+
Client: newFakeClient(cluster),
147130
}
148131

149-
region, err := reconciler.getRegion(context.Background(), template)
132+
region, err := reconciler.getRegion(context.Background(), cluster)
150133

151134
g.Expect(err).To(BeNil())
152135
g.Expect(region).To(Equal(""))
153136
})
154137

155138
t.Run("should return empty when AWSCluster not found", func(t *testing.T) {
156139
g := NewWithT(t)
157-
template := newAWSMachineTemplate("test-template")
158-
template.OwnerReferences = []metav1.OwnerReference{
159-
{
160-
APIVersion: clusterv1.GroupVersion.String(),
161-
Kind: "Cluster",
162-
Name: "test-cluster",
163-
},
164-
}
165140
cluster := &clusterv1.Cluster{
166141
ObjectMeta: metav1.ObjectMeta{
167142
Name: "test-cluster",
@@ -177,10 +152,10 @@ func TestAWSMachineTemplateReconciler(t *testing.T) {
177152
}
178153

179154
reconciler := &AWSMachineTemplateReconciler{
180-
Client: newFakeClient(template, cluster),
155+
Client: newFakeClient(cluster),
181156
}
182157

183-
region, err := reconciler.getRegion(context.Background(), template)
158+
region, err := reconciler.getRegion(context.Background(), cluster)
184159

185160
g.Expect(err).To(BeNil())
186161
g.Expect(region).To(Equal(""))

0 commit comments

Comments
 (0)