Skip to content

Commit 7be10c2

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #12958 from faiq/faiq/fix-controlplane-endpoint
🐛 Stop writing zero values for spec.controlPlaneEndpoint to ControlPlane objects
2 parents e277e49 + c321225 commit 7be10c2

File tree

3 files changed

+196
-0
lines changed

3 files changed

+196
-0
lines changed

‎internal/contract/controlplane.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,33 @@ func (c *ControlPlaneContract) MachineTemplate() *ControlPlaneMachineTemplate {
5454
return &ControlPlaneMachineTemplate{}
5555
}
5656

57+
// IgnorePaths returns a list of paths to be ignored when reconciling an ControlPlane.
58+
// NOTE: The controlPlaneEndpoint struct currently contains two mandatory fields (host and port).
59+
// As the host and port fields are not using omitempty, they are automatically set to their zero values
60+
// if they are not set by the user. We don't want to reconcile the zero values as we would then overwrite
61+
// changes applied by the infrastructure provider controller.
62+
func (c *ControlPlaneContract) IgnorePaths(controlPlane *unstructured.Unstructured) ([]Path, error) {
63+
var ignorePaths []Path
64+
65+
host, ok, err := unstructured.NestedString(controlPlane.UnstructuredContent(), ControlPlane().ControlPlaneEndpoint().host().Path()...)
66+
if err != nil {
67+
return nil, errors.Wrapf(err, "failed to retrieve %s", ControlPlane().ControlPlaneEndpoint().host().Path().String())
68+
}
69+
if ok && host == "" {
70+
ignorePaths = append(ignorePaths, ControlPlane().ControlPlaneEndpoint().host().Path())
71+
}
72+
73+
port, ok, err := unstructured.NestedInt64(controlPlane.UnstructuredContent(), ControlPlane().ControlPlaneEndpoint().port().Path()...)
74+
if err != nil {
75+
return nil, errors.Wrapf(err, "failed to retrieve %s", ControlPlane().ControlPlaneEndpoint().port().Path().String())
76+
}
77+
if ok && port == 0 {
78+
ignorePaths = append(ignorePaths, ControlPlane().ControlPlaneEndpoint().port().Path())
79+
}
80+
81+
return ignorePaths, nil
82+
}
83+
5784
// Version provide access to version field in a ControlPlane object, if any.
5885
// NOTE: When working with unstructured there is no way to understand if the ControlPlane provider
5986
// do support a field in the type definition from the fact that a field is not set in a given instance.

‎internal/contract/controlplane_test.go‎

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,169 @@ func TestControlPlane(t *testing.T) {
487487
})
488488
}
489489

490+
func TestControlPlaneEndpoints(t *testing.T) {
491+
tests := []struct {
492+
name string
493+
controlPlane *unstructured.Unstructured
494+
want []Path
495+
expectErr bool
496+
}{
497+
{
498+
name: "No ignore paths when controlPlaneEndpoint is not set",
499+
controlPlane: &unstructured.Unstructured{
500+
Object: map[string]interface{}{
501+
"spec": map[string]interface{}{
502+
"server": "1.2.3.4",
503+
},
504+
},
505+
},
506+
want: nil,
507+
},
508+
{
509+
name: "No ignore paths when controlPlaneEndpoint is nil",
510+
controlPlane: &unstructured.Unstructured{
511+
Object: map[string]interface{}{
512+
"spec": map[string]interface{}{
513+
"controlPlaneEndpoint": nil,
514+
},
515+
},
516+
},
517+
518+
want: nil,
519+
},
520+
{
521+
name: "No ignore paths when controlPlaneEndpoint is an empty object",
522+
controlPlane: &unstructured.Unstructured{
523+
Object: map[string]interface{}{
524+
"spec": map[string]interface{}{
525+
"controlPlaneEndpoint": map[string]interface{}{},
526+
},
527+
},
528+
},
529+
530+
want: nil,
531+
},
532+
{
533+
name: "Don't ignore host when controlPlaneEndpoint.host is set",
534+
controlPlane: &unstructured.Unstructured{
535+
Object: map[string]interface{}{
536+
"spec": map[string]interface{}{
537+
"controlPlaneEndpoint": map[string]interface{}{
538+
"host": "example.com",
539+
},
540+
},
541+
},
542+
},
543+
want: nil,
544+
},
545+
{
546+
name: "Ignore host when controlPlaneEndpoint.host is set to its zero value",
547+
controlPlane: &unstructured.Unstructured{
548+
Object: map[string]interface{}{
549+
"spec": map[string]interface{}{
550+
"controlPlaneEndpoint": map[string]interface{}{
551+
"host": "",
552+
},
553+
},
554+
},
555+
},
556+
want: []Path{
557+
{"spec", "controlPlaneEndpoint", "host"},
558+
},
559+
},
560+
{
561+
name: "Don't ignore port when controlPlaneEndpoint.port is set",
562+
controlPlane: &unstructured.Unstructured{
563+
Object: map[string]interface{}{
564+
"spec": map[string]interface{}{
565+
"controlPlaneEndpoint": map[string]interface{}{
566+
"port": int64(6443),
567+
},
568+
},
569+
},
570+
},
571+
572+
want: nil,
573+
},
574+
{
575+
name: "Ignore port when controlPlaneEndpoint.port is set to its zero value",
576+
controlPlane: &unstructured.Unstructured{
577+
Object: map[string]interface{}{
578+
"spec": map[string]interface{}{
579+
"controlPlaneEndpoint": map[string]interface{}{
580+
"port": int64(0),
581+
},
582+
},
583+
},
584+
},
585+
want: []Path{
586+
{"spec", "controlPlaneEndpoint", "port"},
587+
},
588+
},
589+
{
590+
name: "Ignore host and port when controlPlaneEndpoint host and port are set to their zero values",
591+
controlPlane: &unstructured.Unstructured{
592+
Object: map[string]interface{}{
593+
"spec": map[string]interface{}{
594+
"controlPlaneEndpoint": map[string]interface{}{
595+
"host": "",
596+
"port": int64(0),
597+
},
598+
},
599+
},
600+
},
601+
want: []Path{
602+
{"spec", "controlPlaneEndpoint", "host"},
603+
{"spec", "controlPlaneEndpoint", "port"},
604+
},
605+
},
606+
{
607+
name: "Ignore host when controlPlaneEndpoint host is to its zero values, even if port is set",
608+
controlPlane: &unstructured.Unstructured{
609+
Object: map[string]interface{}{
610+
"spec": map[string]interface{}{
611+
"controlPlaneEndpoint": map[string]interface{}{
612+
"host": "",
613+
"port": int64(6443),
614+
},
615+
},
616+
},
617+
},
618+
want: []Path{
619+
{"spec", "controlPlaneEndpoint", "host"},
620+
},
621+
},
622+
{
623+
name: "Ignore port when controlPlaneEndpoint port is to its zero values, even if host is set",
624+
controlPlane: &unstructured.Unstructured{
625+
Object: map[string]interface{}{
626+
"spec": map[string]interface{}{
627+
"controlPlaneEndpoint": map[string]interface{}{
628+
"host": "example.com",
629+
"port": int64(0),
630+
},
631+
},
632+
},
633+
},
634+
want: []Path{
635+
{"spec", "controlPlaneEndpoint", "port"},
636+
},
637+
},
638+
}
639+
for _, tt := range tests {
640+
t.Run(tt.name, func(t *testing.T) {
641+
g := NewWithT(t)
642+
got, err := InfrastructureCluster().IgnorePaths(tt.controlPlane)
643+
if tt.expectErr {
644+
g.Expect(err).To(HaveOccurred())
645+
return
646+
}
647+
g.Expect(err).ToNot(HaveOccurred())
648+
g.Expect(got).To(Equal(tt.want))
649+
})
650+
}
651+
}
652+
490653
func TestControlPlaneIsUpgrading(t *testing.T) {
491654
tests := []struct {
492655
name string

‎internal/controllers/topology/cluster/reconcile_state.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,11 +384,17 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope)
384384
// Create or update the ControlPlaneObject for the ControlPlaneState.
385385
log := ctrl.LoggerFrom(ctx).WithValues(s.Desired.ControlPlane.Object.GetKind(), klog.KObj(s.Desired.ControlPlane.Object))
386386
ctx = ctrl.LoggerInto(ctx, log)
387+
388+
ignorePaths, err := contract.ControlPlane().IgnorePaths(s.Desired.ControlPlane.Object)
389+
if err != nil {
390+
return false, errors.Wrap(err, "failed to calculate ignore paths")
391+
}
387392
created, err := r.reconcileReferencedObject(ctx, reconcileReferencedObjectInput{
388393
cluster: s.Current.Cluster,
389394
current: s.Current.ControlPlane.Object,
390395
desired: s.Desired.ControlPlane.Object,
391396
versionGetter: contract.ControlPlane().Version().Get,
397+
ignorePaths: ignorePaths,
392398
})
393399
if err != nil {
394400
// Best effort cleanup of the InfrastructureMachineTemplate (only on creation).

0 commit comments

Comments
 (0)