Merge pull request openshift#1652 from alvaroaleman/add-tests

Add unittests for haproxy config generation
gnufied · Aug 4, 2022 · 06d8f88 · 06d8f88
2 parents 8fdbe5e + 08c44e4
commit 06d8f88
Show file tree

Hide file tree

Showing 9 changed files with 341 additions and 4 deletions.
diff --git a/hypershift-operator/controllers/nodepool/haproxy.go b/hypershift-operator/controllers/nodepool/haproxy.go
@@ -18,7 +18,6 @@ import (
 	"github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster"
 	"github.com/openshift/hypershift/hypershift-operator/controllers/manifests"
 	"github.com/openshift/hypershift/support/config"
-	"github.com/openshift/hypershift/support/releaseinfo"
 	"github.com/openshift/hypershift/support/util"
 	mcfgv1 "github.com/openshift/hypershift/thirdparty/machineconfigoperator/pkg/apis/machineconfiguration.openshift.io/v1"
 	"github.com/vincent-petithory/dataurl"
@@ -60,7 +59,7 @@ func (r *NodePoolReconciler) isHAProxyIgnitionConfigManaged(ctx context.Context,
 	return cpoSkips, controlPlaneOperatorImage, nil
 }
 
-func (r *NodePoolReconciler) reconcileHAProxyIgnitionConfig(ctx context.Context, releaseImage *releaseinfo.ReleaseImage, hcluster *hyperv1.HostedCluster, controlPlaneOperatorImage string) (cfg string, missing bool, err error) {
+func (r *NodePoolReconciler) reconcileHAProxyIgnitionConfig(ctx context.Context, componentImages map[string]string, hcluster *hyperv1.HostedCluster, controlPlaneOperatorImage string) (cfg string, missing bool, err error) {
 	var apiServerExternalAddress string
 	apiServerExternalPort := util.APIPortWithDefaultFromHostedCluster(hcluster, config.DefaultAPIServerPort)
 	if util.IsPrivateHC(hcluster) {
@@ -88,7 +87,7 @@ func (r *NodePoolReconciler) reconcileHAProxyIgnitionConfig(ctx context.Context,
 		apiServerExternalAddress = hostURL.Hostname()
 	}
 
-	haProxyImage, ok := releaseImage.ComponentImages()[haProxyRouterImageName]
+	haProxyImage, ok := componentImages[haProxyRouterImageName]
 	if !ok {
 		return "", true, fmt.Errorf("release image doesn't have a %s image", haProxyRouterImageName)
 	}

diff --git a/hypershift-operator/controllers/nodepool/haproxy_test.go b/hypershift-operator/controllers/nodepool/haproxy_test.go
@@ -1,9 +1,20 @@
 package nodepool
 
 import (
+	"context"
+	"strings"
 	"testing"
 
+	ignitionapi "github.com/coreos/ignition/v2/config/v3_2/types"
+	hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
 	"github.com/openshift/hypershift/support/testutil"
+	mcfgv1 "github.com/openshift/hypershift/thirdparty/machineconfigoperator/pkg/apis/machineconfiguration.openshift.io/v1"
+	"github.com/vincent-petithory/dataurl"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	utilpointer "k8s.io/utils/pointer"
+	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/yaml"
 )
 
@@ -21,3 +32,156 @@ func TestAPIServerHAProxyConfig(t *testing.T) {
 	}
 	testutil.CompareWithFixture(t, yamlConfig)
 }
+
+func TestReconcileHAProxyIgnitionConfig(t *testing.T) {
+	hc := func(m ...func(*hyperv1.HostedCluster)) *hyperv1.HostedCluster {
+		hc := &hyperv1.HostedCluster{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "hc",
+				Namespace: "clusters",
+			},
+			Spec: hyperv1.HostedClusterSpec{
+				Platform: hyperv1.PlatformSpec{
+					Type: hyperv1.AWSPlatform,
+					AWS:  &hyperv1.AWSPlatformSpec{},
+				},
+			},
+		}
+		for _, m := range m {
+			m(hc)
+		}
+		return hc
+	}
+	const kubeconfig = `apiVersion: v1
+clusters:
+- cluster:
+    server: https://kubeconfig-host:6443
+  name: cluster
+contexts:
+- context:
+    cluster: cluster
+    user: ""
+    namespace: default
+  name: cluster
+current-context: cluster
+kind: Config`
+	testCases := []struct {
+		name                         string
+		hc                           *hyperv1.HostedCluster
+		other                        []crclient.Object
+		expectedHAProxyConfigContent []string
+	}{
+		{
+			name: "private cluster uses .local address",
+			hc: hc(func(hc *hyperv1.HostedCluster) {
+				hc.Spec.Platform.AWS.EndpointAccess = hyperv1.Private
+			}),
+
+			expectedHAProxyConfigContent: []string{"api." + hc().Name + ".hypershift.local:6443"},
+		},
+		{
+			name: "private cluster uses .local address and custom apiserver port",
+			hc: hc(func(hc *hyperv1.HostedCluster) {
+				hc.Spec.Platform.AWS.EndpointAccess = hyperv1.Private
+				hc.Spec.Networking.APIServer = &hyperv1.APIServerNetworking{Port: utilpointer.Int32Ptr(443)}
+			}),
+
+			expectedHAProxyConfigContent: []string{"api." + hc().Name + ".hypershift.local:443"},
+		},
+		{
+			name: "public and private cluster uses .local address",
+			hc: hc(func(hc *hyperv1.HostedCluster) {
+				hc.Spec.Platform.AWS.EndpointAccess = hyperv1.PublicAndPrivate
+			}),
+
+			expectedHAProxyConfigContent: []string{"api." + hc().Name + ".hypershift.local:6443"},
+		},
+		{
+			name: "public and private cluster uses .local address and custom apiserver port",
+			hc: hc(func(hc *hyperv1.HostedCluster) {
+				hc.Spec.Platform.AWS.EndpointAccess = hyperv1.PublicAndPrivate
+				hc.Spec.Networking.APIServer = &hyperv1.APIServerNetworking{Port: utilpointer.Int32Ptr(443)}
+			}),
+
+			expectedHAProxyConfigContent: []string{"api." + hc().Name + ".hypershift.local:443"},
+		},
+		{
+			name: "public cluster uses address from kubeconfig",
+			hc: hc(func(hc *hyperv1.HostedCluster) {
+				hc.Spec.Platform.AWS.EndpointAccess = hyperv1.Public
+				hc.Status.KubeConfig = &corev1.LocalObjectReference{Name: "kk"}
+			}),
+			other: []crclient.Object{&corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "kk", Namespace: hc().Namespace},
+				Data: map[string][]byte{
+					"kubeconfig": []byte(kubeconfig),
+				},
+			}},
+
+			expectedHAProxyConfigContent: []string{"kubeconfig-host:6443"},
+		},
+		{
+			name: "public cluster uses address from kubeconfig and custom port",
+			hc: hc(func(hc *hyperv1.HostedCluster) {
+				hc.Spec.Platform.AWS.EndpointAccess = hyperv1.Public
+				hc.Spec.Networking.APIServer = &hyperv1.APIServerNetworking{Port: utilpointer.Int32Ptr(443)}
+				hc.Status.KubeConfig = &corev1.LocalObjectReference{Name: "kk"}
+			}),
+			other: []crclient.Object{&corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "kk", Namespace: hc().Namespace},
+				Data: map[string][]byte{
+					"kubeconfig": []byte(kubeconfig),
+				},
+			}},
+
+			expectedHAProxyConfigContent: []string{"kubeconfig-host:443"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+
+			r := &NodePoolReconciler{
+				Client: fake.NewClientBuilder().WithObjects(tc.other...).Build(),
+			}
+			cfg, _, err := r.reconcileHAProxyIgnitionConfig(context.Background(),
+				map[string]string{"haproxy-router": "some-image"},
+				tc.hc,
+				"cpo-image",
+			)
+			if err != nil {
+				t.Fatalf("reconcileHaProxyIgnitionConfig: %v", err)
+			}
+
+			mcfg := &mcfgv1.MachineConfig{}
+			if err := yaml.Unmarshal([]byte(cfg), mcfg); err != nil {
+				t.Fatalf("cannot unmarshal machine config: %v", err)
+			}
+			ignitionCfg := &ignitionapi.Config{}
+			if err := yaml.Unmarshal(mcfg.Spec.Config.Raw, ignitionCfg); err != nil {
+				t.Fatalf("cannot unmarshal ignition config: %v", err)
+			}
+
+			var haproxyConfig *dataurl.DataURL
+			for _, file := range ignitionCfg.Storage.Files {
+				if file.Path == "/etc/kubernetes/apiserver-proxy-config/haproxy.cfg" {
+					haproxyConfig, err = dataurl.DecodeString(*file.Contents.Source)
+					if err != nil {
+						t.Fatalf("cannot decode dataurl: %v", err)
+					}
+				}
+			}
+			if haproxyConfig == nil {
+				t.Fatalf("Couldn't find haproxy config in ignition config %s", string(mcfg.Spec.Config.Raw))
+			}
+
+			for _, line := range tc.expectedHAProxyConfigContent {
+				if !strings.Contains(string(haproxyConfig.Data), line) {
+					t.Errorf("expected %s in %s", line, string(haproxyConfig.Data))
+				}
+			}
+
+			testutil.CompareWithFixture(t, haproxyConfig.Data)
+		})
+	}
+}
diff --git a/hypershift-operator/controllers/nodepool/nodepool_controller.go b/hypershift-operator/controllers/nodepool/nodepool_controller.go
@@ -1131,7 +1131,7 @@ func (r *NodePoolReconciler) getConfig(ctx context.Context,
 		}
 		expectedCoreConfigResources--
 
-		haproxyIgnitionConfig, missing, err := r.reconcileHAProxyIgnitionConfig(ctx, releaseImage, hcluster, cpoImage)
+		haproxyIgnitionConfig, missing, err := r.reconcileHAProxyIgnitionConfig(ctx, releaseImage.ComponentImages(), hcluster, cpoImage)
 		if err != nil {
 			return "", false, fmt.Errorf("failed to generate haporoxy ignition config: %w", err)
 		}

diff --git a/...ta/zz_fixture_TestReconcileHAProxyIgnitionConfig_private_cluster_uses_.local_address.yaml b/...ta/zz_fixture_TestReconcileHAProxyIgnitionConfig_private_cluster_uses_.local_address.yaml
@@ -0,0 +1,29 @@
+global
+  maxconn 7000
+  log stdout local0
+  log stdout local1 notice
+
+defaults
+  mode tcp
+  timeout client 10m
+  timeout server 10m
+  timeout connect 10s
+  timeout client-fin 5s
+  timeout server-fin 5s
+  timeout queue 5s
+  retries 3
+
+frontend local_apiserver
+  bind 172.20.0.1:6443
+  log global
+  mode tcp
+  option tcplog
+  default_backend remote_apiserver
+
+backend remote_apiserver
+  mode tcp
+  log global
+  option httpchk GET /version
+  option log-health-checks
+  default-server inter 10s fall 3 rise 3
+  server controlplane api.hc.hypershift.local:6443
diff --git a/...eHAProxyIgnitionConfig_private_cluster_uses_.local_address_and_custom_apiserver_port.yaml b/...eHAProxyIgnitionConfig_private_cluster_uses_.local_address_and_custom_apiserver_port.yaml
@@ -0,0 +1,29 @@
+global
+  maxconn 7000
+  log stdout local0
+  log stdout local1 notice
+
+defaults
+  mode tcp
+  timeout client 10m
+  timeout server 10m
+  timeout connect 10s
+  timeout client-fin 5s
+  timeout server-fin 5s
+  timeout queue 5s
+  retries 3
+
+frontend local_apiserver
+  bind 172.20.0.1:443
+  log global
+  mode tcp
+  option tcplog
+  default_backend remote_apiserver
+
+backend remote_apiserver
+  mode tcp
+  log global
+  option httpchk GET /version
+  option log-health-checks
+  default-server inter 10s fall 3 rise 3
+  server controlplane api.hc.hypershift.local:443
diff --git a/...re_TestReconcileHAProxyIgnitionConfig_public_and_private_cluster_uses_.local_address.yaml b/...re_TestReconcileHAProxyIgnitionConfig_public_and_private_cluster_uses_.local_address.yaml
@@ -0,0 +1,29 @@
+global
+  maxconn 7000
+  log stdout local0
+  log stdout local1 notice
+
+defaults
+  mode tcp
+  timeout client 10m
+  timeout server 10m
+  timeout connect 10s
+  timeout client-fin 5s
+  timeout server-fin 5s
+  timeout queue 5s
+  retries 3
+
+frontend local_apiserver
+  bind 172.20.0.1:6443
+  log global
+  mode tcp
+  option tcplog
+  default_backend remote_apiserver
+
+backend remote_apiserver
+  mode tcp
+  log global
+  option httpchk GET /version
+  option log-health-checks
+  default-server inter 10s fall 3 rise 3
+  server controlplane api.hc.hypershift.local:6443
diff --git a/...itionConfig_public_and_private_cluster_uses_.local_address_and_custom_apiserver_port.yaml b/...itionConfig_public_and_private_cluster_uses_.local_address_and_custom_apiserver_port.yaml
@@ -0,0 +1,29 @@
+global
+  maxconn 7000
+  log stdout local0
+  log stdout local1 notice
+
+defaults
+  mode tcp
+  timeout client 10m
+  timeout server 10m
+  timeout connect 10s
+  timeout client-fin 5s
+  timeout server-fin 5s
+  timeout queue 5s
+  retries 3
+
+frontend local_apiserver
+  bind 172.20.0.1:443
+  log global
+  mode tcp
+  option tcplog
+  default_backend remote_apiserver
+
+backend remote_apiserver
+  mode tcp
+  log global
+  option httpchk GET /version
+  option log-health-checks
+  default-server inter 10s fall 3 rise 3
+  server controlplane api.hc.hypershift.local:443
diff --git a/...xture_TestReconcileHAProxyIgnitionConfig_public_cluster_uses_address_from_kubeconfig.yaml b/...xture_TestReconcileHAProxyIgnitionConfig_public_cluster_uses_address_from_kubeconfig.yaml
@@ -0,0 +1,29 @@
+global
+  maxconn 7000
+  log stdout local0
+  log stdout local1 notice
+
+defaults
+  mode tcp
+  timeout client 10m
+  timeout server 10m
+  timeout connect 10s
+  timeout client-fin 5s
+  timeout server-fin 5s
+  timeout queue 5s
+  retries 3
+
+frontend local_apiserver
+  bind 172.20.0.1:6443
+  log global
+  mode tcp
+  option tcplog
+  default_backend remote_apiserver
+
+backend remote_apiserver
+  mode tcp
+  log global
+  option httpchk GET /version
+  option log-health-checks
+  default-server inter 10s fall 3 rise 3
+  server controlplane kubeconfig-host:6443
diff --git a/...ileHAProxyIgnitionConfig_public_cluster_uses_address_from_kubeconfig_and_custom_port.yaml b/...ileHAProxyIgnitionConfig_public_cluster_uses_address_from_kubeconfig_and_custom_port.yaml
@@ -0,0 +1,29 @@
+global
+  maxconn 7000
+  log stdout local0
+  log stdout local1 notice
+
+defaults
+  mode tcp
+  timeout client 10m
+  timeout server 10m
+  timeout connect 10s
+  timeout client-fin 5s
+  timeout server-fin 5s
+  timeout queue 5s
+  retries 3
+
+frontend local_apiserver
+  bind 172.20.0.1:443
+  log global
+  mode tcp
+  option tcplog
+  default_backend remote_apiserver
+
+backend remote_apiserver
+  mode tcp
+  log global
+  option httpchk GET /version
+  option log-health-checks
+  default-server inter 10s fall 3 rise 3
+  server controlplane kubeconfig-host:443