Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Avoid redundant reconciles if only generation of Paused condition changed & bump CAPI to main (17th March, 9e7afa749358) #71

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ linters-settings:
- pkg: sigs.k8s.io/cluster-api/exp/api/v1beta1
alias: expv1
# CAPI exp addons
- pkg: sigs.k8s.io/cluster-api/exp/addons/api/v1beta1
- pkg: sigs.k8s.io/cluster-api/api/addons/v1beta1
alias: addonsv1
# CAPI IPAM
- pkg: sigs.k8s.io/cluster-api/exp/ipam/api/v1beta1
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ IMPORT_BOSS_VER := v0.28.1
IMPORT_BOSS := $(abspath $(TOOLS_BIN_DIR)/$(IMPORT_BOSS_BIN))
IMPORT_BOSS_PKG := k8s.io/code-generator/cmd/import-boss

CAPI_HACK_TOOLS_VER := ccaea78cdbf068a1fe072bde78794e98c15c1072 # Note: this a commit ID of from CAPI main, supposed to be in v1.10
CAPI_HACK_TOOLS_VER := 9e7afa749358adc7e0c6ae5408d0d8eeb2affa0a # Note: this a commit ID of from CAPI main, supposed to be in v1.10

BOSKOSCTL_BIN := boskosctl
BOSKOSCTL := $(abspath $(TOOLS_BIN_DIR)/$(BOSKOSCTL_BIN))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,18 +407,23 @@ spec:
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.
Expand All @@ -428,6 +433,8 @@ spec:
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -799,18 +799,23 @@ spec:
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.
Expand All @@ -820,6 +825,8 @@ spec:
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,18 +324,23 @@ spec:
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.
Expand All @@ -345,6 +350,8 @@ spec:
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1510,10 +1510,18 @@ spec:
properties:
address:
description: address is the machine address.
maxLength: 256
minLength: 1
type: string
type:
description: type is the machine address type, one of Hostname,
ExternalIP, InternalIP, ExternalDNS or InternalDNS.
enum:
- Hostname
- ExternalIP
- InternalIP
- ExternalDNS
- InternalDNS
type: string
required:
- address
Expand All @@ -1537,18 +1545,23 @@ spec:
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.
Expand All @@ -1558,6 +1571,8 @@ spec:
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1592,18 +1592,23 @@ spec:
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.
Expand All @@ -1613,6 +1618,8 @@ spec:
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ spec:
properties:
host:
description: host is the hostname on which the API server is serving.
maxLength: 512
type: string
port:
description: port is the port on which the API server is serving.
Expand Down Expand Up @@ -76,18 +77,23 @@ spec:
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.
Expand All @@ -97,6 +103,8 @@ spec:
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ spec:
host:
description: host is the hostname on which the API server
is serving.
maxLength: 512
type: string
port:
description: port is the port on which the API server
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,18 +196,23 @@ spec:
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.
Expand All @@ -217,6 +222,8 @@ spec:
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
Expand Down
9 changes: 5 additions & 4 deletions controllers/vmware/vspherecluster_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,16 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
ctx = ctrl.LoggerInto(ctx, log)
}

if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, r.Client, cluster, vsphereCluster); err != nil || isPaused || conditionChanged {
return ctrl.Result{}, err
}

// Build the patch helper.
patchHelper, err := patch.NewHelper(vsphereCluster, r.Client)
if err != nil {
return reconcile.Result{}, err
}

if isPaused, requeue, err := paused.EnsurePausedCondition(ctx, r.Client, cluster, vsphereCluster); err != nil || isPaused || requeue {
return ctrl.Result{}, err
}

// Build the cluster context.
clusterContext := &vmware.ClusterContext{
Cluster: cluster,
Expand Down Expand Up @@ -185,6 +185,7 @@ func (r *ClusterReconciler) patch(ctx context.Context, clusterCtx *vmware.Cluste
vmwarev1.LoadBalancerReadyCondition,
}},
patch.WithOwnedV1Beta2Conditions{Conditions: []string{
clusterv1.PausedV1Beta2Condition,
vmwarev1.VSphereClusterReadyV1Beta2Condition,
vmwarev1.VSphereClusterResourcePolicyReadyV1Beta2Condition,
vmwarev1.VSphereClusterNetworkReadyV1Beta2Condition,
Expand Down
8 changes: 4 additions & 4 deletions controllers/vspherecluster_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,16 @@ func (r *clusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
ctx = ctrl.LoggerInto(ctx, log)
}

if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, r.Client, cluster, vsphereCluster); err != nil || isPaused || conditionChanged {
return ctrl.Result{}, err
}

// Create the patch helper.
patchHelper, err := patch.NewHelper(vsphereCluster, r.Client)
if err != nil {
return reconcile.Result{}, err
}

if isPaused, requeue, err := paused.EnsurePausedCondition(ctx, r.Client, cluster, vsphereCluster); err != nil || isPaused || requeue {
return ctrl.Result{}, err
}

// Create the cluster context for this request.
clusterContext := &capvcontext.ClusterContext{
Cluster: cluster,
Expand Down
12 changes: 7 additions & 5 deletions controllers/vsphereclusteridentity_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,20 +87,22 @@ func (r clusterIdentityReconciler) Reconcile(ctx context.Context, req reconcile.
return ctrl.Result{}, err
}

if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, r.Client, nil, identity); err != nil || isPaused || conditionChanged {
return ctrl.Result{}, err
}

// Create the patch helper.
patchHelper, err := patch.NewHelper(identity, r.Client)
if err != nil {
return reconcile.Result{}, err
}

if isPaused, requeue, err := paused.EnsurePausedCondition(ctx, r.Client, nil, identity); err != nil || isPaused || requeue {
return ctrl.Result{}, err
}

defer func() {
conditions.SetSummary(identity, conditions.WithConditions(infrav1.CredentialsAvailableCondidtion))

if err := patchHelper.Patch(ctx, identity); err != nil {
if err := patchHelper.Patch(ctx, identity, patch.WithOwnedV1Beta2Conditions{Conditions: []string{
clusterv1.PausedV1Beta2Condition,
}}); err != nil {
reterr = kerrors.NewAggregate([]error{reterr, err})
}
}()
Expand Down
8 changes: 4 additions & 4 deletions controllers/vspheredeploymentzone_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,15 @@ func (r vsphereDeploymentZoneReconciler) Reconcile(ctx context.Context, request
return ctrl.Result{}, err
}

if isPaused, conditionChanged, err := paused.EnsurePausedCondition(ctx, r.Client, nil, vsphereDeploymentZone); err != nil || isPaused || conditionChanged {
return ctrl.Result{}, err
}

patchHelper, err := patch.NewHelper(vsphereDeploymentZone, r.Client)
if err != nil {
return reconcile.Result{}, err
}

if isPaused, requeue, err := paused.EnsurePausedCondition(ctx, r.Client, nil, vsphereDeploymentZone); err != nil || isPaused || requeue {
return ctrl.Result{}, err
}

vsphereDeploymentZoneContext := &capvcontext.VSphereDeploymentZoneContext{
ControllerManagerContext: r.ControllerManagerContext,
VSphereDeploymentZone: vsphereDeploymentZone,
Expand Down
Loading