From 1d940cefbb1516a7e1dfdbd2d898e292705a5fe1 Mon Sep 17 00:00:00 2001 From: razashahid107 Date: Tue, 10 Oct 2023 17:27:02 +0500 Subject: [PATCH 1/5] added function to update control plane endpoint --- controllers/hetznercluster_controller.go | 55 +++++++++++++----------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/controllers/hetznercluster_controller.go b/controllers/hetznercluster_controller.go index e12178fa3..e483fed98 100644 --- a/controllers/hetznercluster_controller.go +++ b/controllers/hetznercluster_controller.go @@ -198,31 +198,9 @@ func (r *HetznerClusterReconciler) reconcileNormal(ctx context.Context, clusterS if err := placementgroup.NewService(clusterScope).Reconcile(ctx); err != nil { return reconcile.Result{}, fmt.Errorf("failed to reconcile placement groups for HetznerCluster %s/%s: %w", hetznerCluster.Namespace, hetznerCluster.Name, err) } - - if hetznerCluster.Spec.ControlPlaneLoadBalancer.Enabled { - if hetznerCluster.Status.ControlPlaneLoadBalancer.IPv4 != "" { - defaultHost := hetznerCluster.Status.ControlPlaneLoadBalancer.IPv4 - defaultPort := int32(hetznerCluster.Spec.ControlPlaneLoadBalancer.Port) - - if hetznerCluster.Spec.ControlPlaneEndpoint == nil { - hetznerCluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{ - Host: defaultHost, - Port: defaultPort, - } - } else { - if hetznerCluster.Spec.ControlPlaneEndpoint.Host == "" { - hetznerCluster.Spec.ControlPlaneEndpoint.Host = defaultHost - } - if hetznerCluster.Spec.ControlPlaneEndpoint.Port == 0 { - hetznerCluster.Spec.ControlPlaneEndpoint.Port = defaultPort - } - } - - hetznerCluster.Status.Ready = true - } - } else if hetznerCluster.Spec.ControlPlaneEndpoint != nil { - hetznerCluster.Status.Ready = true - } + + // update control plane endpoint + hetznerCluster.Status.Ready = SetControlPlaneEndpoint(hetznerCluster) // delete deprecated conditions of old clusters conditions.Delete(clusterScope.HetznerCluster, infrav1.DeprecatedHetznerClusterTargetClusterReadyCondition) @@ -260,6 +238,33 @@ func (r *HetznerClusterReconciler) reconcileNormal(ctx context.Context, clusterS return reconcile.Result{}, nil } +func SetControlPlaneEndpoint(hetznerCluster *infrav1.HetznerCluster) (bool) { + if hetznerCluster.Spec.ControlPlaneLoadBalancer.Enabled { + if hetznerCluster.Status.ControlPlaneLoadBalancer.IPv4 != "" { + defaultHost := hetznerCluster.Status.ControlPlaneLoadBalancer.IPv4 + defaultPort := int32(hetznerCluster.Spec.ControlPlaneLoadBalancer.Port) + + if hetznerCluster.Spec.ControlPlaneEndpoint == nil { + hetznerCluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{ + Host: defaultHost, + Port: defaultPort, + } + } else { + if hetznerCluster.Spec.ControlPlaneEndpoint.Host == "" { + hetznerCluster.Spec.ControlPlaneEndpoint.Host = defaultHost + } + if hetznerCluster.Spec.ControlPlaneEndpoint.Port == 0 { + hetznerCluster.Spec.ControlPlaneEndpoint.Port = defaultPort + } + } + return true + } + } else if hetznerCluster.Spec.ControlPlaneEndpoint != nil { + return true + } + return false +} + func (r *HetznerClusterReconciler) reconcileDelete(ctx context.Context, clusterScope *scope.ClusterScope) (reconcile.Result, error) { hetznerCluster := clusterScope.HetznerCluster From 7c71490f46a1529e3b6d591ebe7be2dceb003cab Mon Sep 17 00:00:00 2001 From: razashahid107 Date: Thu, 12 Oct 2023 18:40:46 +0500 Subject: [PATCH 2/5] Added test for SetControlPlaneEndpoint() function --- controllers/hetznercluster_controller_test.go | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/controllers/hetznercluster_controller_test.go b/controllers/hetznercluster_controller_test.go index 9f3f53c34..b72ead8bb 100644 --- a/controllers/hetznercluster_controller_test.go +++ b/controllers/hetznercluster_controller_test.go @@ -18,11 +18,13 @@ package controllers import ( "context" + "testing" "time" "github.com/hetznercloud/hcloud-go/v2/hcloud" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -840,3 +842,31 @@ var _ = Describe("reconcileRateLimit", func() { Expect(reconcileRateLimit(hetznerCluster, testEnv.RateLimitWaitTime)).To(BeFalse()) }) }) + +func TestSetControlPlaneEndpoint(t *testing.T) { + type args struct { + hetznerCluster *infrav1.HetznerCluster + } + tests := []struct{ + name string + args args + want bool + }{ + { + name: "should return true", + args: args{hetznerCluster: new(infrav1.HetznerCluster)}, + want: true, + }, + { + name: "should return false", + args: args{hetznerCluster: new(infrav1.HetznerCluster)}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T){ + got := SetControlPlaneEndpoint(tt.args.hetznerCluster) + assert.Equal(t, tt.want, got) + }) + } +} \ No newline at end of file From 5858e835879729cabb812582d7457c31b232d06c Mon Sep 17 00:00:00 2001 From: razashahid107 Date: Fri, 13 Oct 2023 19:46:48 +0500 Subject: [PATCH 3/5] Added unit test in hetznercluster_controller_test.go, made minor change to hetznercluster_controller.go --- controllers/hetznercluster_controller.go | 3 ++- controllers/hetznercluster_controller_test.go | 26 ++++++++++++------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/controllers/hetznercluster_controller.go b/controllers/hetznercluster_controller.go index e483fed98..40dc72615 100644 --- a/controllers/hetznercluster_controller.go +++ b/controllers/hetznercluster_controller.go @@ -249,6 +249,7 @@ func SetControlPlaneEndpoint(hetznerCluster *infrav1.HetznerCluster) (bool) { Host: defaultHost, Port: defaultPort, } + return true } else { if hetznerCluster.Spec.ControlPlaneEndpoint.Host == "" { hetznerCluster.Spec.ControlPlaneEndpoint.Host = defaultHost @@ -256,8 +257,8 @@ func SetControlPlaneEndpoint(hetznerCluster *infrav1.HetznerCluster) (bool) { if hetznerCluster.Spec.ControlPlaneEndpoint.Port == 0 { hetznerCluster.Spec.ControlPlaneEndpoint.Port = defaultPort } + return true } - return true } } else if hetznerCluster.Spec.ControlPlaneEndpoint != nil { return true diff --git a/controllers/hetznercluster_controller_test.go b/controllers/hetznercluster_controller_test.go index b72ead8bb..12594093e 100644 --- a/controllers/hetznercluster_controller_test.go +++ b/controllers/hetznercluster_controller_test.go @@ -844,28 +844,36 @@ var _ = Describe("reconcileRateLimit", func() { }) func TestSetControlPlaneEndpoint(t *testing.T) { - type args struct { - hetznerCluster *infrav1.HetznerCluster - } tests := []struct{ name string - args args + hetznerCluster *infrav1.HetznerCluster want bool }{ { - name: "should return true", - args: args{hetznerCluster: new(infrav1.HetznerCluster)}, + name: "the function should return true if Loadbalancer is enabled, IPV4 does not include a string and the ControlPlaneEndpoint is set to nil", + hetznerCluster: &infrav1.HetznerCluster{Spec: infrav1.HetznerClusterSpec{ControlPlaneLoadBalancer: infrav1.LoadBalancerSpec{Enabled: true}, ControlPlaneEndpoint: nil}, Status: infrav1.HetznerClusterStatus{ControlPlaneLoadBalancer: &infrav1.LoadBalancerStatus{IPv4: "xyz"}}}, + want: true, + }, + { + name: "the function should return true if Loadbalancer is enabled, IPV4 does not include a string and the ControlPlaneEndpoint is NOT set to nil", + hetznerCluster: &infrav1.HetznerCluster{Spec: infrav1.HetznerClusterSpec{ControlPlaneLoadBalancer: infrav1.LoadBalancerSpec{Enabled: true}, ControlPlaneEndpoint: &clusterv1.APIEndpoint{Host: "", Port: 0}}, Status: infrav1.HetznerClusterStatus{ControlPlaneLoadBalancer: &infrav1.LoadBalancerStatus{IPv4: "xyz"}}}, want: true, }, { - name: "should return false", - args: args{hetznerCluster: new(infrav1.HetznerCluster)}, + name: "the function should return true if Loadbalancer is enabled, IPV4 does not include a string and the ControlPlaneEndpoint contains preset values in its Host and Port component", + hetznerCluster: &infrav1.HetznerCluster{Spec: infrav1.HetznerClusterSpec{ControlPlaneLoadBalancer: infrav1.LoadBalancerSpec{Enabled: true}, ControlPlaneEndpoint: &clusterv1.APIEndpoint{Host: "xyz", Port: 1}}, Status: infrav1.HetznerClusterStatus{ControlPlaneLoadBalancer: &infrav1.LoadBalancerStatus{IPv4: "xyz"}}}, + want: true, + }, + { + name: "the function should return false if Loadbalancer is not enabled and the ControlPlaneEndpoint is nil", + hetznerCluster: &infrav1.HetznerCluster{Spec: infrav1.HetznerClusterSpec{ControlPlaneLoadBalancer: infrav1.LoadBalancerSpec{Enabled: false}, ControlPlaneEndpoint: nil}}, want: false, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T){ - got := SetControlPlaneEndpoint(tt.args.hetznerCluster) + got := SetControlPlaneEndpoint(tt.hetznerCluster) assert.Equal(t, tt.want, got) }) } From 1c49b5cc374b7496ca9b85b04035643c4e2e4d5b Mon Sep 17 00:00:00 2001 From: razashahid107 Date: Mon, 16 Oct 2023 12:21:51 +0500 Subject: [PATCH 4/5] Rephrased names of test --- controllers/hetznercluster_controller_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/hetznercluster_controller_test.go b/controllers/hetznercluster_controller_test.go index 12594093e..7bab674aa 100644 --- a/controllers/hetznercluster_controller_test.go +++ b/controllers/hetznercluster_controller_test.go @@ -850,12 +850,12 @@ func TestSetControlPlaneEndpoint(t *testing.T) { want bool }{ { - name: "the function should return true if Loadbalancer is enabled, IPV4 does not include a string and the ControlPlaneEndpoint is set to nil", + name: "the function should return true if Loadbalancer is enabled, IPV4 does not include a string and the ControlPlaneEndpoint is set to nil. This will enable changes to the ControlPlaneEndpoint's host and port", hetznerCluster: &infrav1.HetznerCluster{Spec: infrav1.HetznerClusterSpec{ControlPlaneLoadBalancer: infrav1.LoadBalancerSpec{Enabled: true}, ControlPlaneEndpoint: nil}, Status: infrav1.HetznerClusterStatus{ControlPlaneLoadBalancer: &infrav1.LoadBalancerStatus{IPv4: "xyz"}}}, want: true, }, { - name: "the function should return true if Loadbalancer is enabled, IPV4 does not include a string and the ControlPlaneEndpoint is NOT set to nil", + name: "the function should return true if Loadbalancer is enabled, IPV4 does not include a string and the ControlPlaneEndpoint is NOT set to nil. This testcase will enable changes to the ControlPlaneEndpoint's host and port", hetznerCluster: &infrav1.HetznerCluster{Spec: infrav1.HetznerClusterSpec{ControlPlaneLoadBalancer: infrav1.LoadBalancerSpec{Enabled: true}, ControlPlaneEndpoint: &clusterv1.APIEndpoint{Host: "", Port: 0}}, Status: infrav1.HetznerClusterStatus{ControlPlaneLoadBalancer: &infrav1.LoadBalancerStatus{IPv4: "xyz"}}}, want: true, }, From cd1749dc9870a2d6b7e701539834e4f359731880 Mon Sep 17 00:00:00 2001 From: razashahid107 Date: Mon, 16 Oct 2023 23:12:37 +0500 Subject: [PATCH 5/5] Added condition to check Host and Port in unit test --- controllers/hetznercluster_controller_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/controllers/hetznercluster_controller_test.go b/controllers/hetznercluster_controller_test.go index 7bab674aa..2fdfd8e84 100644 --- a/controllers/hetznercluster_controller_test.go +++ b/controllers/hetznercluster_controller_test.go @@ -875,6 +875,12 @@ func TestSetControlPlaneEndpoint(t *testing.T) { t.Run(tt.name, func(t *testing.T){ got := SetControlPlaneEndpoint(tt.hetznerCluster) assert.Equal(t, tt.want, got) + if tt.hetznerCluster.Status.ControlPlaneLoadBalancer.IPv4 != "" { + // check the Host + assert.Equal(t, tt.hetznerCluster.Status.ControlPlaneLoadBalancer.IPv4, tt.hetznerCluster.Spec.ControlPlaneEndpoint.Host) + // check the Port + assert.Equal(t, tt.hetznerCluster.Spec.ControlPlaneLoadBalancer.Port, tt.hetznerCluster.Spec.ControlPlaneEndpoint.Port) + } }) } } \ No newline at end of file