Skip to content

Commit 49e535f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #12525 from sbueringer/pr-remove-healthcheck-pointers
⚠️ Remove pointers from ClusterClass and Cluster healthCheck fields
2 parents d43e0ac + 96f99a8 commit 49e535f

File tree

26 files changed

+356
-253
lines changed

26 files changed

+356
-253
lines changed

‎.golangci-kal.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ linters:
196196
linters:
197197
- kubeapilinter
198198
- path: "api/core/v1beta2/*|api/controlplane/kubeadm/v1beta2/*"
199-
text: "optionalfields: field (Naming|MachineNaming|Deletion) is optional and should (be a pointer|have the omitempty tag)"
199+
text: "optionalfields: field (Naming|HealthCheck|MachineNaming|Deletion) is optional and should (be a pointer|have the omitempty tag)"
200200
linters:
201201
- kubeapilinter
202202

‎api/controlplane/kubeadm/v1beta1/conversion_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ func spokeKubeadmControlPlane(in *KubeadmControlPlane, c randfill.Continue) {
167167
if reflect.DeepEqual(in.Spec.RolloutBefore, &RolloutBefore{}) {
168168
in.Spec.RolloutBefore = nil
169169
}
170+
if in.Spec.RolloutStrategy != nil {
171+
if reflect.DeepEqual(in.Spec.RolloutStrategy.RollingUpdate, &RollingUpdate{}) {
172+
in.Spec.RolloutStrategy.RollingUpdate = nil
173+
}
174+
}
170175
if reflect.DeepEqual(in.Spec.RolloutStrategy, &RolloutStrategy{}) {
171176
in.Spec.RolloutStrategy = nil
172177
}
@@ -252,6 +257,11 @@ func spokeKubeadmControlPlaneTemplate(in *KubeadmControlPlaneTemplate, c randfil
252257
if reflect.DeepEqual(in.Spec.Template.Spec.RolloutBefore, &RolloutBefore{}) {
253258
in.Spec.Template.Spec.RolloutBefore = nil
254259
}
260+
if in.Spec.Template.Spec.RolloutStrategy != nil {
261+
if reflect.DeepEqual(in.Spec.Template.Spec.RolloutStrategy.RollingUpdate, &RollingUpdate{}) {
262+
in.Spec.Template.Spec.RolloutStrategy.RollingUpdate = nil
263+
}
264+
}
255265
if reflect.DeepEqual(in.Spec.Template.Spec.RolloutStrategy, &RolloutStrategy{}) {
256266
in.Spec.Template.Spec.RolloutStrategy = nil
257267
}

‎api/core/v1beta1/conversion.go

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,6 @@ func Convert_v1beta1_ControlPlaneClass_To_v1beta2_ControlPlaneClass(in *ControlP
677677
convert_v1beta1_LocalObjectTemplate_To_v1beta2_ClusterClassTemplateReference(&in.LocalObjectTemplate, &out.TemplateRef, s)
678678

679679
if in.MachineHealthCheck != nil {
680-
out.HealthCheck = &clusterv1.ControlPlaneClassHealthCheck{}
681680
for _, c := range in.MachineHealthCheck.UnhealthyConditions {
682681
out.HealthCheck.Checks.UnhealthyNodeConditions = append(out.HealthCheck.Checks.UnhealthyNodeConditions, clusterv1.UnhealthyNodeCondition{
683682
Type: c.Type,
@@ -712,7 +711,7 @@ func Convert_v1beta2_ControlPlaneClass_To_v1beta1_ControlPlaneClass(in *clusterv
712711
out.NodeDeletionTimeout = clusterv1.ConvertFromSeconds(in.Deletion.NodeDeletionTimeoutSeconds)
713712
Convert_v1beta2_ClusterClassTemplateReference_To_v1beta1_LocalObjectTemplate(&in.TemplateRef, &out.LocalObjectTemplate, s)
714713

715-
if in.HealthCheck != nil {
714+
if !reflect.DeepEqual(in.HealthCheck, clusterv1.ControlPlaneClassHealthCheck{}) {
716715
out.MachineHealthCheck = &MachineHealthCheckClass{}
717716
for _, c := range in.HealthCheck.Checks.UnhealthyNodeConditions {
718717
out.MachineHealthCheck.UnhealthyConditions = append(out.MachineHealthCheck.UnhealthyConditions, UnhealthyCondition{
@@ -755,7 +754,6 @@ func Convert_v1beta1_ControlPlaneTopology_To_v1beta2_ControlPlaneTopology(in *Co
755754
out.Deletion.NodeDeletionTimeoutSeconds = clusterv1.ConvertToSeconds(in.NodeDeletionTimeout)
756755

757756
if in.MachineHealthCheck != nil {
758-
out.HealthCheck = &clusterv1.ControlPlaneTopologyHealthCheck{}
759757
out.HealthCheck.Enabled = in.MachineHealthCheck.Enable
760758
for _, c := range in.MachineHealthCheck.UnhealthyConditions {
761759
out.HealthCheck.Checks.UnhealthyNodeConditions = append(out.HealthCheck.Checks.UnhealthyNodeConditions, clusterv1.UnhealthyNodeCondition{
@@ -791,7 +789,7 @@ func Convert_v1beta2_ControlPlaneTopology_To_v1beta1_ControlPlaneTopology(in *cl
791789
out.NodeVolumeDetachTimeout = clusterv1.ConvertFromSeconds(in.Deletion.NodeVolumeDetachTimeoutSeconds)
792790
out.NodeDeletionTimeout = clusterv1.ConvertFromSeconds(in.Deletion.NodeDeletionTimeoutSeconds)
793791

794-
if in.HealthCheck != nil {
792+
if !reflect.DeepEqual(in.HealthCheck, clusterv1.ControlPlaneTopologyHealthCheck{}) {
795793
out.MachineHealthCheck = &MachineHealthCheckTopology{}
796794
out.MachineHealthCheck.Enable = in.HealthCheck.Enabled
797795
for _, c := range in.HealthCheck.Checks.UnhealthyNodeConditions {
@@ -839,9 +837,6 @@ func Convert_v1beta1_MachineDeploymentClass_To_v1beta2_MachineDeploymentClass(in
839837
}
840838
}
841839
if in.Strategy.Remediation != nil && in.Strategy.Remediation.MaxInFlight != nil {
842-
if out.HealthCheck == nil {
843-
out.HealthCheck = &clusterv1.MachineDeploymentClassHealthCheck{}
844-
}
845840
out.HealthCheck.Remediation.MaxInFlight = in.Strategy.Remediation.MaxInFlight
846841
}
847842
}
@@ -850,9 +845,6 @@ func Convert_v1beta1_MachineDeploymentClass_To_v1beta2_MachineDeploymentClass(in
850845
out.Deletion.NodeDeletionTimeoutSeconds = clusterv1.ConvertToSeconds(in.NodeDeletionTimeout)
851846

852847
if in.MachineHealthCheck != nil {
853-
if out.HealthCheck == nil {
854-
out.HealthCheck = &clusterv1.MachineDeploymentClassHealthCheck{}
855-
}
856848
for _, c := range in.MachineHealthCheck.UnhealthyConditions {
857849
out.HealthCheck.Checks.UnhealthyNodeConditions = append(out.HealthCheck.Checks.UnhealthyNodeConditions, clusterv1.UnhealthyNodeCondition{
858850
Type: c.Type,
@@ -909,7 +901,7 @@ func Convert_v1beta2_MachineDeploymentClass_To_v1beta1_MachineDeploymentClass(in
909901
}
910902
out.Strategy.RollingUpdate.DeletePolicy = ptr.To(string(in.Deletion.Order))
911903
}
912-
if in.HealthCheck != nil && in.HealthCheck.Remediation.MaxInFlight != nil {
904+
if in.HealthCheck.Remediation.MaxInFlight != nil {
913905
if out.Strategy == nil {
914906
out.Strategy = &MachineDeploymentStrategy{}
915907
}
@@ -923,7 +915,7 @@ func Convert_v1beta2_MachineDeploymentClass_To_v1beta1_MachineDeploymentClass(in
923915
out.NodeDeletionTimeout = clusterv1.ConvertFromSeconds(in.Deletion.NodeDeletionTimeoutSeconds)
924916

925917
// Check if HealthCheck is not nil and another field apart from MaxInFlight is set (MaxInFlight is set in Strategy above).
926-
if in.HealthCheck != nil && !reflect.DeepEqual(in.HealthCheck, &clusterv1.MachineDeploymentClassHealthCheck{
918+
if !reflect.DeepEqual(in.HealthCheck, clusterv1.MachineDeploymentClassHealthCheck{
927919
Remediation: clusterv1.MachineDeploymentClassHealthCheckRemediation{MaxInFlight: in.HealthCheck.Remediation.MaxInFlight},
928920
}) {
929921
out.MachineHealthCheck = &MachineHealthCheckClass{}
@@ -973,9 +965,6 @@ func Convert_v1beta1_MachineDeploymentTopology_To_v1beta2_MachineDeploymentTopol
973965
}
974966
}
975967
if in.Strategy.Remediation != nil && in.Strategy.Remediation.MaxInFlight != nil {
976-
if out.HealthCheck == nil {
977-
out.HealthCheck = &clusterv1.MachineDeploymentTopologyHealthCheck{}
978-
}
979968
out.HealthCheck.Remediation.MaxInFlight = in.Strategy.Remediation.MaxInFlight
980969
}
981970
}
@@ -984,9 +973,6 @@ func Convert_v1beta1_MachineDeploymentTopology_To_v1beta2_MachineDeploymentTopol
984973
out.Deletion.NodeDeletionTimeoutSeconds = clusterv1.ConvertToSeconds(in.NodeDeletionTimeout)
985974

986975
if in.MachineHealthCheck != nil {
987-
if out.HealthCheck == nil {
988-
out.HealthCheck = &clusterv1.MachineDeploymentTopologyHealthCheck{}
989-
}
990976
out.HealthCheck.Enabled = in.MachineHealthCheck.Enable
991977
for _, c := range in.MachineHealthCheck.UnhealthyConditions {
992978
out.HealthCheck.Checks.UnhealthyNodeConditions = append(out.HealthCheck.Checks.UnhealthyNodeConditions, clusterv1.UnhealthyNodeCondition{
@@ -1036,7 +1022,7 @@ func Convert_v1beta2_MachineDeploymentTopology_To_v1beta1_MachineDeploymentTopol
10361022
}
10371023
out.Strategy.RollingUpdate.DeletePolicy = ptr.To(string(in.Deletion.Order))
10381024
}
1039-
if in.HealthCheck != nil && in.HealthCheck.Remediation.MaxInFlight != nil {
1025+
if in.HealthCheck.Remediation.MaxInFlight != nil {
10401026
if out.Strategy == nil {
10411027
out.Strategy = &MachineDeploymentStrategy{}
10421028
}
@@ -1050,7 +1036,7 @@ func Convert_v1beta2_MachineDeploymentTopology_To_v1beta1_MachineDeploymentTopol
10501036
out.NodeDeletionTimeout = clusterv1.ConvertFromSeconds(in.Deletion.NodeDeletionTimeoutSeconds)
10511037

10521038
// Check if HealthCheck is not nil and another field apart from MaxInFlight is set (MaxInFlight is set in Strategy above).
1053-
if in.HealthCheck != nil && !reflect.DeepEqual(in.HealthCheck, &clusterv1.MachineDeploymentTopologyHealthCheck{
1039+
if !reflect.DeepEqual(in.HealthCheck, clusterv1.MachineDeploymentTopologyHealthCheck{
10541040
Remediation: clusterv1.MachineDeploymentTopologyHealthCheckRemediation{MaxInFlight: in.HealthCheck.Remediation.MaxInFlight},
10551041
}) {
10561042
out.MachineHealthCheck = &MachineHealthCheckTopology{}

‎api/core/v1beta2/cluster_types.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ type ControlPlaneTopology struct {
614614
// healthCheck allows to enable, disable and override control plane health check
615615
// configuration from the ClusterClass for this control plane.
616616
// +optional
617-
HealthCheck *ControlPlaneTopologyHealthCheck `json:"healthCheck,omitempty"`
617+
HealthCheck ControlPlaneTopologyHealthCheck `json:"healthCheck,omitempty,omitzero"`
618618

619619
// deletion contains configuration options for Machine deletion.
620620
// +optional
@@ -689,7 +689,7 @@ type ControlPlaneTopologyHealthCheck struct {
689689
Remediation ControlPlaneTopologyHealthCheckRemediation `json:"remediation,omitempty,omitzero"`
690690
}
691691

692-
// IsDefined returns true if both checks and remediation are zero.
692+
// IsDefined returns true if one of checks and remediation are not zero.
693693
func (m *ControlPlaneTopologyHealthCheck) IsDefined() bool {
694694
return !reflect.ValueOf(m.Checks).IsZero() || !reflect.ValueOf(m.Remediation).IsZero()
695695
}
@@ -852,7 +852,7 @@ type MachineDeploymentTopology struct {
852852
// healthCheck allows to enable, disable and override MachineDeployment health check
853853
// configuration from the ClusterClass for this MachineDeployment.
854854
// +optional
855-
HealthCheck *MachineDeploymentTopologyHealthCheck `json:"healthCheck,omitempty"`
855+
HealthCheck MachineDeploymentTopologyHealthCheck `json:"healthCheck,omitempty,omitzero"`
856856

857857
// deletion contains configuration options for Machine deletion.
858858
// +optional
@@ -938,7 +938,7 @@ type MachineDeploymentTopologyHealthCheck struct {
938938
Remediation MachineDeploymentTopologyHealthCheckRemediation `json:"remediation,omitempty,omitzero"`
939939
}
940940

941-
// IsDefined returns true if both checks and remediation are zero.
941+
// IsDefined returns true if one of checks and remediation are not zero.
942942
func (m *MachineDeploymentTopologyHealthCheck) IsDefined() bool {
943943
return !reflect.ValueOf(m.Checks).IsZero() || !reflect.ValueOf(m.Remediation).IsZero()
944944
}

‎api/core/v1beta2/clusterclass_types.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ limitations under the License.
1717
package v1beta2
1818

1919
import (
20+
"reflect"
21+
2022
corev1 "k8s.io/api/core/v1"
2123
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2224
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -171,7 +173,7 @@ type ControlPlaneClass struct {
171173
// This field is supported if and only if the ControlPlane provider template
172174
// referenced above is Machine based and supports setting replicas.
173175
// +optional
174-
HealthCheck *ControlPlaneClassHealthCheck `json:"healthCheck,omitempty"`
176+
HealthCheck ControlPlaneClassHealthCheck `json:"healthCheck,omitempty,omitzero"`
175177

176178
// naming allows changing the naming pattern used when creating the control plane provider object.
177179
// +optional
@@ -226,6 +228,11 @@ type ControlPlaneClassHealthCheck struct {
226228
Remediation ControlPlaneClassHealthCheckRemediation `json:"remediation,omitempty,omitzero"`
227229
}
228230

231+
// IsDefined returns true if one of checks and remediation are not zero.
232+
func (m *ControlPlaneClassHealthCheck) IsDefined() bool {
233+
return !reflect.ValueOf(m.Checks).IsZero() || !reflect.ValueOf(m.Remediation).IsZero()
234+
}
235+
229236
// ControlPlaneClassHealthCheckChecks are the checks that are used to evaluate if a control plane Machine is healthy.
230237
// +kubebuilder:validation:MinProperties=1
231238
type ControlPlaneClassHealthCheckChecks struct {
@@ -405,7 +412,7 @@ type MachineDeploymentClass struct {
405412

406413
// healthCheck defines a MachineHealthCheck for this MachineDeploymentClass.
407414
// +optional
408-
HealthCheck *MachineDeploymentClassHealthCheck `json:"healthCheck,omitempty"`
415+
HealthCheck MachineDeploymentClassHealthCheck `json:"healthCheck,omitempty,omitzero"`
409416

410417
// failureDomain is the failure domain the machines will be created in.
411418
// Must match the name of a FailureDomain from the Cluster status.
@@ -480,6 +487,11 @@ type MachineDeploymentClassHealthCheck struct {
480487
Remediation MachineDeploymentClassHealthCheckRemediation `json:"remediation,omitempty,omitzero"`
481488
}
482489

490+
// IsDefined returns true if one of checks and remediation are not zero.
491+
func (m *MachineDeploymentClassHealthCheck) IsDefined() bool {
492+
return !reflect.ValueOf(m.Checks).IsZero() || !reflect.ValueOf(m.Remediation).IsZero()
493+
}
494+
483495
// MachineDeploymentClassHealthCheckChecks are the checks that are used to evaluate if a MachineDeployment Machine is healthy.
484496
// +kubebuilder:validation:MinProperties=1
485497
type MachineDeploymentClassHealthCheckChecks struct {

‎api/core/v1beta2/zz_generated.deepcopy.go

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

‎api/core/v1beta2/zz_generated.openapi.go

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

‎exp/topology/desiredstate/desired_state.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3232
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3333
"k8s.io/apimachinery/pkg/runtime/schema"
34-
"k8s.io/apimachinery/pkg/util/intstr"
3534
"k8s.io/klog/v2"
3635
"k8s.io/utils/ptr"
3736
ctrl "sigs.k8s.io/controller-runtime"
@@ -821,11 +820,8 @@ func (g *generator) computeMachineDeployment(ctx context.Context, s *scope.Scope
821820
}
822821
}
823822

824-
var remediationMaxInFlight *intstr.IntOrString
825-
if machineDeploymentClass.HealthCheck != nil && machineDeploymentClass.HealthCheck.Remediation.MaxInFlight != nil {
826-
remediationMaxInFlight = machineDeploymentClass.HealthCheck.Remediation.MaxInFlight
827-
}
828-
if machineDeploymentTopology.HealthCheck != nil && machineDeploymentTopology.HealthCheck.Remediation.MaxInFlight != nil {
823+
remediationMaxInFlight := machineDeploymentClass.HealthCheck.Remediation.MaxInFlight
824+
if machineDeploymentTopology.HealthCheck.Remediation.MaxInFlight != nil {
829825
remediationMaxInFlight = machineDeploymentTopology.HealthCheck.Remediation.MaxInFlight
830826
}
831827

‎exp/topology/desiredstate/desired_state_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,7 +1757,7 @@ func TestComputeMachineDeployment(t *testing.T) {
17571757
clusterClassMDStrategy := clusterv1.MachineDeploymentRolloutStrategy{
17581758
Type: clusterv1.OnDeleteMachineDeploymentStrategyType,
17591759
}
1760-
clusterClassHealthCheck := &clusterv1.MachineDeploymentClassHealthCheck{
1760+
clusterClassHealthCheck := clusterv1.MachineDeploymentClassHealthCheck{
17611761
Remediation: clusterv1.MachineDeploymentClassHealthCheckRemediation{
17621762
MaxInFlight: ptr.To(intstr.FromInt32(5)),
17631763
},
@@ -1771,7 +1771,7 @@ func TestComputeMachineDeployment(t *testing.T) {
17711771
WithAnnotations(annotations).
17721772
WithInfrastructureTemplate(workerInfrastructureMachineTemplate).
17731773
WithBootstrapTemplate(workerBootstrapTemplate).
1774-
WithMachineHealthCheckClass(&clusterv1.MachineDeploymentClassHealthCheck{
1774+
WithMachineHealthCheckClass(clusterv1.MachineDeploymentClassHealthCheck{
17751775
Checks: clusterv1.MachineDeploymentClassHealthCheckChecks{
17761776
UnhealthyNodeConditions: unhealthyNodeConditions,
17771777
NodeStartupTimeoutSeconds: nodeTimeoutDuration,
@@ -1816,7 +1816,7 @@ func TestComputeMachineDeployment(t *testing.T) {
18161816
},
18171817
BootstrapTemplate: workerBootstrapTemplate,
18181818
InfrastructureMachineTemplate: workerInfrastructureMachineTemplate,
1819-
HealthCheck: &clusterv1.MachineDeploymentClassHealthCheck{
1819+
HealthCheck: clusterv1.MachineDeploymentClassHealthCheck{
18201820
Checks: clusterv1.MachineDeploymentClassHealthCheckChecks{
18211821
UnhealthyNodeConditions: unhealthyNodeConditions,
18221822
NodeStartupTimeoutSeconds: ptr.To(int32(1)),
@@ -1836,7 +1836,7 @@ func TestComputeMachineDeployment(t *testing.T) {
18361836
topologyMDStrategy := clusterv1.MachineDeploymentRolloutStrategy{
18371837
Type: clusterv1.RollingUpdateMachineDeploymentStrategyType,
18381838
}
1839-
topologyHealthCheck := &clusterv1.MachineDeploymentTopologyHealthCheck{
1839+
topologyHealthCheck := clusterv1.MachineDeploymentTopologyHealthCheck{
18401840
Remediation: clusterv1.MachineDeploymentTopologyHealthCheckRemediation{
18411841
MaxInFlight: ptr.To(intstr.FromInt32(1)),
18421842
},
@@ -1992,7 +1992,7 @@ func TestComputeMachineDeployment(t *testing.T) {
19921992
},
19931993
BootstrapTemplate: workerBootstrapTemplate,
19941994
InfrastructureMachineTemplate: workerInfrastructureMachineTemplate,
1995-
HealthCheck: &clusterv1.MachineDeploymentClassHealthCheck{
1995+
HealthCheck: clusterv1.MachineDeploymentClassHealthCheck{
19961996
Checks: clusterv1.MachineDeploymentClassHealthCheckChecks{
19971997
UnhealthyNodeConditions: unhealthyNodeConditions,
19981998
NodeStartupTimeoutSeconds: ptr.To(int32(1)),

0 commit comments

Comments
 (0)