From 00395108a38d20f67a6fc168ed1eed129ee5a1c0 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Wed, 18 Sep 2024 14:36:26 +0200 Subject: [PATCH 1/2] Update to the streamlined version of ToolchainCluster and fix the register member tests to take the new version and workflows into account. --- pkg/cmd/adm/register_member.go | 25 +---- pkg/cmd/adm/register_member_test.go | 139 ++++++++++++++-------------- pkg/test/toolchaincluster.go | 4 +- 3 files changed, 73 insertions(+), 95 deletions(-) diff --git a/pkg/cmd/adm/register_member.go b/pkg/cmd/adm/register_member.go index 51cde5c..c72f7f7 100644 --- a/pkg/cmd/adm/register_member.go +++ b/pkg/cmd/adm/register_member.go @@ -179,7 +179,6 @@ func (v *registerMemberValidated) addCluster(ctx *extendedCommandContext, source ctx.Printlnf("creating secret %s/%s in the %s cluster", targetClusterDetails.namespace, secretName, sourceClusterType.TheOtherType()) kubeConfigSecret := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: targetClusterDetails.namespace}} _, err = controllerutil.CreateOrUpdate(context.TODO(), targetClusterDetails.client, kubeConfigSecret, func() error { - // update the secret label labels := kubeConfigSecret.GetLabels() if labels == nil { @@ -196,7 +195,6 @@ func (v *registerMemberValidated) addCluster(ctx *extendedCommandContext, source return nil }) - if err != nil { return err } @@ -210,24 +208,9 @@ func (v *registerMemberValidated) addCluster(ctx *extendedCommandContext, source ctx.Printlnf("creating ToolchainCluster representation of %s in %s:", sourceClusterType, targetClusterDetails.toolchainClusterName) toolchainClusterCR := &toolchainv1alpha1.ToolchainCluster{ObjectMeta: metav1.ObjectMeta{Name: sourceClusterDetails.toolchainClusterName, Namespace: targetClusterDetails.namespace}} _, err = controllerutil.CreateOrUpdate(context.TODO(), targetClusterDetails.client, toolchainClusterCR, func() error { - - // update the tc label - labels := toolchainClusterCR.GetLabels() - if labels == nil { - labels = make(map[string]string) - } - // TODO drop this "namespace" label as soon as ToolchainCluster controller supports loading data from kubeconfig - labels["namespace"] = sourceClusterDetails.namespace - toolchainClusterCR.Labels = labels - toolchainClusterCR.Spec.APIEndpoint = sourceClusterDetails.apiEndpoint toolchainClusterCR.Spec.SecretRef.Name = secretName - if insecureSkipTLSVerify { - toolchainClusterCR.Spec.DisabledTLSValidations = []toolchainv1alpha1.TLSValidation{toolchainv1alpha1.TLSAll} - } - return nil }) - if err != nil { return err } @@ -346,7 +329,7 @@ func getToolchainClustersWithHostname(ctx context.Context, cl runtimeclient.Clie clusters := []toolchainv1alpha1.ToolchainCluster{} for _, tc := range list.Items { - if tc.Spec.APIEndpoint == hostName { + if tc.Status.APIEndpoint == hostName { clusters = append(clusters, tc) } } @@ -428,8 +411,8 @@ func validateArgs(ctx *extendedCommandContext, args registerMemberArgs) (*regist if len(hostsInMember.Items) > 1 { errors = append(errors, fmt.Sprintf("member misconfigured: the member cluster (%s) is already registered with more than 1 host in namespace %s", memberApiEndpoint, args.memberNamespace)) } else if len(hostsInMember.Items) == 1 { - if hostsInMember.Items[0].Spec.APIEndpoint != hostApiEndpoint { - errors = append(errors, fmt.Sprintf("the member is already registered with another host (%s) so registering it with the new one (%s) would result in an invalid configuration", hostsInMember.Items[0].Spec.APIEndpoint, hostApiEndpoint)) + if hostsInMember.Items[0].Status.APIEndpoint != hostApiEndpoint { + errors = append(errors, fmt.Sprintf("the member is already registered with another host (%s) so registering it with the new one (%s) would result in an invalid configuration", hostsInMember.Items[0].Status.APIEndpoint, hostApiEndpoint)) } if hostsInMember.Items[0].Name != hostToolchainClusterName { errors = append(errors, fmt.Sprintf("the host is already in the member namespace using a ToolchainCluster object with the name '%s' but the new registration would use a ToolchainCluster with the name '%s' which would lead to an invalid configuration", hostsInMember.Items[0].Name, hostToolchainClusterName)) @@ -528,7 +511,7 @@ until the SpaceProvisionerConfig.spec.enabled is set to true. func findToolchainClusterForMember(allToolchainClusters []toolchainv1alpha1.ToolchainCluster, memberAPIEndpoint, memberOperatorNamespace string) *toolchainv1alpha1.ToolchainCluster { for _, tc := range allToolchainClusters { - if tc.Spec.APIEndpoint == memberAPIEndpoint && tc.Labels["namespace"] == memberOperatorNamespace { + if tc.Status.APIEndpoint == memberAPIEndpoint && tc.Status.OperatorNamespace == memberOperatorNamespace { return &tc } } diff --git a/pkg/cmd/adm/register_member_test.go b/pkg/cmd/adm/register_member_test.go index db52b8c..0afb769 100644 --- a/pkg/cmd/adm/register_member_test.go +++ b/pkg/cmd/adm/register_member_test.go @@ -35,16 +35,19 @@ func TestRegisterMember(t *testing.T) { hostKubeconfig := PersistKubeConfigFile(t, HostKubeConfig()) memberKubeconfig := PersistKubeConfigFile(t, MemberKubeConfig()) + hostNs := "toolchain-host-operator" + memberNs := "toolchain-member-operator" + toolchainClusterMemberSa := corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: "toolchaincluster-member", - Namespace: "toolchain-member-operator", + Namespace: memberNs, }, } toolchainClusterHostSa := corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: "toolchaincluster-host", - Namespace: "toolchain-host-operator", + Namespace: hostNs, }, } @@ -62,7 +65,7 @@ func TestRegisterMember(t *testing.T) { term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) // force the ready condition on the toolchaincluster created ( this is done by the tc controller in prod env ) - mockCreateToolchainClusterWithReadyCondition(fakeClient) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) ctx := newExtendedCommandContext(term, newClient) expectedExampleSPC := &toolchainv1alpha1.SpaceProvisionerConfig{ @@ -72,7 +75,7 @@ func TestRegisterMember(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{ Name: "member-cool-server.com", - Namespace: "toolchain-host-operator", + Namespace: hostNs, }, Spec: toolchainv1alpha1.SpaceProvisionerConfigSpec{ ToolchainCluster: "member-cool-server.com", @@ -90,16 +93,16 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, err) // check the expected secrets are there with the kubeconfigs // the member kubeconfig secret in the host namespace - verifyToolchainClusterSecret(t, fakeClient, toolchainClusterMemberSa.Name, "toolchain-host-operator", "toolchain-member-operator", memberToolchainClusterName) + verifyToolchainClusterSecret(t, fakeClient, toolchainClusterMemberSa.Name, hostNs, memberNs, memberToolchainClusterName) // the host secret in the member namespace - verifyToolchainClusterSecret(t, fakeClient, toolchainClusterHostSa.Name, "toolchain-member-operator", "toolchain-host-operator", hostToolchainClusterName) + verifyToolchainClusterSecret(t, fakeClient, toolchainClusterHostSa.Name, memberNs, hostNs, hostToolchainClusterName) tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-host-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) assert.Len(t, tcs.Items, 1) assert.Equal(t, memberToolchainClusterName, tcs.Items[0].Name) // secret ref in tc matches assert.Equal(t, toolchainClusterMemberSa.Name+"-"+memberToolchainClusterName, tcs.Items[0].Spec.SecretRef.Name) - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-member-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) assert.Len(t, tcs.Items, 1) assert.Equal(t, hostToolchainClusterName, tcs.Items[0].Name) // secret ref in tc matches @@ -113,7 +116,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterInNamespaceWithReadyCondition(fakeClient, "toolchain-member-operator") // we set to ready only the host toolchaincluster in member operator namespace + mockCreateToolchainClusterInNamespaceWithReadyCondition(t, fakeClient, memberNs, hostNs, memberNs) // we set to ready only the host toolchaincluster in member operator namespace ctx := newExtendedCommandContext(term, newClient) // when @@ -122,9 +125,9 @@ func TestRegisterMember(t *testing.T) { // then require.Error(t, err) tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-host-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) assert.Len(t, tcs.Items, 1) - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-member-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) assert.Len(t, tcs.Items, 1) assert.Contains(t, term.Output(), "The ToolchainCluster resource representing the member in the host cluster has not become ready.") }) @@ -133,7 +136,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterInNamespaceWithReadyCondition(fakeClient, "toolchain-host-operator") // set to ready only the member toolchaincluster in host operator namespace + mockCreateToolchainClusterInNamespaceWithReadyCondition(t, fakeClient, hostNs, hostNs, memberNs) // set to ready only the member toolchaincluster in host operator namespace ctx := newExtendedCommandContext(term, newClient) // when @@ -142,10 +145,10 @@ func TestRegisterMember(t *testing.T) { // then require.Error(t, err) tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-host-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) assert.Empty(t, tcs.Items) assert.Contains(t, term.Output(), "The ToolchainCluster resource representing the host in the member cluster has not become ready.") - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-member-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) assert.Len(t, tcs.Items, 1) }) @@ -153,7 +156,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(fakeClient) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) ctx := newExtendedCommandContext(term, newClient) // when @@ -169,7 +172,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(fakeClient) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) ctx := newExtendedCommandContext(term, newClient) // when @@ -185,17 +188,14 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(fakeClient) ctx := newExtendedCommandContext(term, newClient) preexistingToolchainCluster := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "member-cool-server.com", - Namespace: "toolchain-host-operator", - }, - Spec: toolchainv1alpha1.ToolchainClusterSpec{ - APIEndpoint: "https://cool-server.com", + Namespace: hostNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ + APIEndpoint: "https://cool-server.com", Conditions: []toolchainv1alpha1.Condition{ { Type: toolchainv1alpha1.ConditionReady, @@ -208,6 +208,8 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster.Name = "member-cool-server.com1" require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster.DeepCopy())) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) + // when err := registerMemberCluster(ctx, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "2")) @@ -225,7 +227,7 @@ func TestRegisterMember(t *testing.T) { term1 := NewFakeTerminalWithResponse("Y") term2 := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(fakeClient) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) ctx1 := newExtendedCommandContext(term1, newClient) ctx2 := newExtendedCommandContext(term2, newClient) @@ -248,7 +250,7 @@ func TestRegisterMember(t *testing.T) { term1 := NewFakeTerminalWithResponse("Y") term2 := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(fakeClient) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) ctx1 := newExtendedCommandContext(term1, newClient) ctx2 := newExtendedCommandContext(term2, newClient) @@ -272,17 +274,15 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(fakeClient) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) ctx := newExtendedCommandContext(term, newClient) preexistingToolchainCluster1 := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "host-not-so-cool-server.com", - Namespace: "toolchain-member-operator", - }, - Spec: toolchainv1alpha1.ToolchainClusterSpec{ - APIEndpoint: "https://not-so-cool-server.com", + Namespace: memberNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ + APIEndpoint: "https://not-so-cool-server.com", Conditions: []toolchainv1alpha1.Condition{ { Type: toolchainv1alpha1.ConditionReady, @@ -294,12 +294,10 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster2 := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "host-uncool-server.com", - Namespace: "toolchain-member-operator", - }, - Spec: toolchainv1alpha1.ToolchainClusterSpec{ - APIEndpoint: "https://uncool-server.com", + Namespace: memberNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ + APIEndpoint: "https://uncool-server.com", Conditions: []toolchainv1alpha1.Condition{ { Type: toolchainv1alpha1.ConditionReady, @@ -329,12 +327,10 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "host-not-so-cool-server.com", - Namespace: "toolchain-member-operator", - }, - Spec: toolchainv1alpha1.ToolchainClusterSpec{ - APIEndpoint: "https://not-so-cool-server.com", + Namespace: memberNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ + APIEndpoint: "https://not-so-cool-server.com", Conditions: []toolchainv1alpha1.Condition{ { Type: toolchainv1alpha1.ConditionReady, @@ -363,12 +359,10 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "host-with-weird-name", - Namespace: "toolchain-member-operator", - }, - Spec: toolchainv1alpha1.ToolchainClusterSpec{ - APIEndpoint: "https://cool-server.com", + Namespace: memberNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ + APIEndpoint: "https://cool-server.com", Conditions: []toolchainv1alpha1.Condition{ { Type: toolchainv1alpha1.ConditionReady, @@ -397,15 +391,11 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "member-with-weird-name", - Namespace: "toolchain-host-operator", - Labels: map[string]string{ - "namespace": "toolchain-member-operator", - }, - }, - Spec: toolchainv1alpha1.ToolchainClusterSpec{ - APIEndpoint: "https://cool-server.com", + Namespace: hostNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ + APIEndpoint: "https://cool-server.com", + OperatorNamespace: memberNs, Conditions: []toolchainv1alpha1.Condition{ { Type: toolchainv1alpha1.ConditionReady, @@ -430,7 +420,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterHostSa) // we pre-provision only the host toolchaincluster ServiceAccount - mockCreateToolchainClusterWithReadyCondition(fakeClient) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) ctx := newExtendedCommandContext(term, newClient) // when @@ -440,9 +430,9 @@ func TestRegisterMember(t *testing.T) { require.Error(t, err) assert.Contains(t, term.Output(), "The toolchain-member-operator/toolchaincluster-member ServiceAccount is not present in the member cluster.") tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-host-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) assert.Empty(t, tcs.Items) - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-member-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) assert.Len(t, tcs.Items, 1) }) @@ -459,47 +449,54 @@ func TestRegisterMember(t *testing.T) { require.Error(t, err) assert.Contains(t, term.Output(), "The toolchain-host-operator/toolchaincluster-host ServiceAccount is not present in the host cluster.") tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-host-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) assert.Empty(t, tcs.Items) - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace("toolchain-member-operator"))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) assert.Empty(t, tcs.Items) }) } -func mockCreateToolchainClusterInNamespaceWithReadyCondition(fakeClient *test.FakeClient, namespace string) { +func mockCreateToolchainClusterInNamespaceWithReadyCondition(t *testing.T, fakeClient *test.FakeClient, namespace string, hostOperatorNamespace, memberOperatorNamespace string) { fakeClient.MockCreate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.CreateOption) error { if obj, ok := obj.(*toolchainv1alpha1.ToolchainCluster); ok { if obj.GetNamespace() == namespace { - obj.Status = toolchainv1alpha1.ToolchainClusterStatus{ - Conditions: []toolchainv1alpha1.Condition{ - { - Type: toolchainv1alpha1.ConditionReady, - Status: corev1.ConditionTrue, - }, - }, - } + _mockCreateToolchainClusterWithReadyConditionModify(t, obj, hostOperatorNamespace, memberOperatorNamespace) } } return fakeClient.Client.Create(ctx, obj, opts...) } } -func mockCreateToolchainClusterWithReadyCondition(fakeClient *test.FakeClient) { +func mockCreateToolchainClusterWithReadyCondition(t *testing.T, fakeClient *test.FakeClient, hostOperatorNamespace, memberOperatorNamespace string) { fakeClient.MockCreate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.CreateOption) error { if obj, ok := obj.(*toolchainv1alpha1.ToolchainCluster); ok { - obj.Status = toolchainv1alpha1.ToolchainClusterStatus{ - Conditions: []toolchainv1alpha1.Condition{ - { - Type: toolchainv1alpha1.ConditionReady, - Status: corev1.ConditionTrue, - }, - }, - } + _mockCreateToolchainClusterWithReadyConditionModify(t, obj, hostOperatorNamespace, memberOperatorNamespace) } return fakeClient.Client.Create(ctx, obj, opts...) } } +func _mockCreateToolchainClusterWithReadyConditionModify(t *testing.T, obj *toolchainv1alpha1.ToolchainCluster, hostOperatorNamespace, memberOperatorNamespace string) { + var operatorNamespace string + if obj.GetNamespace() == hostOperatorNamespace { + operatorNamespace = memberOperatorNamespace + } else if obj.GetNamespace() == memberOperatorNamespace { + operatorNamespace = hostOperatorNamespace + } else { + assert.Fail(t, "the mock create of ToolchainCluster only works in host operator namespace %s or member operator namespace %s but the creation of toolchain cluster was requested in %s", hostOperatorNamespace, memberOperatorNamespace, obj.GetNamespace()) + } + obj.Status = toolchainv1alpha1.ToolchainClusterStatus{ + APIEndpoint: "https://cool-server.com", + OperatorNamespace: operatorNamespace, + Conditions: []toolchainv1alpha1.Condition{ + { + Type: toolchainv1alpha1.ConditionReady, + Status: corev1.ConditionTrue, + }, + }, + } +} + func verifyToolchainClusterSecret(t *testing.T, fakeClient *test.FakeClient, saName, secretNamespace, ctxNamespace, tcName string) { secret := &corev1.Secret{} secretName := fmt.Sprintf("%s-%s", saName, tcName) @@ -538,7 +535,7 @@ func newFakeClientsFromRestConfig(t *testing.T, initObjs ...runtime.Object) (new return fakeClient.Client.Update(ctx, obj, opts...) } return func(cfg *rest.Config) (runtimeclient.Client, error) { - assert.Contains(t, cfg.Host, "http") + assert.Contains(t, cfg.Host, "https") assert.Contains(t, cfg.Host, "://") assert.Contains(t, cfg.Host, ".com") return fakeClient, nil diff --git a/pkg/test/toolchaincluster.go b/pkg/test/toolchaincluster.go index 27683f2..dee2fc7 100644 --- a/pkg/test/toolchaincluster.go +++ b/pkg/test/toolchaincluster.go @@ -19,10 +19,8 @@ func NewToolchainCluster(modifiers ...ToolchainClusterModifier) *toolchainv1alph Name: "member1", Namespace: test.HostOperatorNs, }, - Spec: toolchainv1alpha1.ToolchainClusterSpec{ + Status: toolchainv1alpha1.ToolchainClusterStatus{ APIEndpoint: "https://api.member.com:6443", - CABundle: "somebundle", - SecretRef: toolchainv1alpha1.LocalSecretReference{}, }, } for _, modify := range modifiers { From 9e008d9e314dd9cd013248b596005e563357d4a4 Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Thu, 3 Oct 2024 15:24:36 +0200 Subject: [PATCH 2/2] Use the predefined host and member operator namespaces from the test package and simplify the logic in the mocking functions slightly. --- pkg/cmd/adm/register_member_test.go | 91 ++++++++++++++--------------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/pkg/cmd/adm/register_member_test.go b/pkg/cmd/adm/register_member_test.go index 0afb769..453e3dc 100644 --- a/pkg/cmd/adm/register_member_test.go +++ b/pkg/cmd/adm/register_member_test.go @@ -35,19 +35,16 @@ func TestRegisterMember(t *testing.T) { hostKubeconfig := PersistKubeConfigFile(t, HostKubeConfig()) memberKubeconfig := PersistKubeConfigFile(t, MemberKubeConfig()) - hostNs := "toolchain-host-operator" - memberNs := "toolchain-member-operator" - toolchainClusterMemberSa := corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: "toolchaincluster-member", - Namespace: memberNs, + Namespace: test.MemberOperatorNs, }, } toolchainClusterHostSa := corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: "toolchaincluster-host", - Namespace: hostNs, + Namespace: test.HostOperatorNs, }, } @@ -65,7 +62,7 @@ func TestRegisterMember(t *testing.T) { term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) // force the ready condition on the toolchaincluster created ( this is done by the tc controller in prod env ) - mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) ctx := newExtendedCommandContext(term, newClient) expectedExampleSPC := &toolchainv1alpha1.SpaceProvisionerConfig{ @@ -75,7 +72,7 @@ func TestRegisterMember(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{ Name: "member-cool-server.com", - Namespace: hostNs, + Namespace: test.HostOperatorNs, }, Spec: toolchainv1alpha1.SpaceProvisionerConfigSpec{ ToolchainCluster: "member-cool-server.com", @@ -93,16 +90,16 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, err) // check the expected secrets are there with the kubeconfigs // the member kubeconfig secret in the host namespace - verifyToolchainClusterSecret(t, fakeClient, toolchainClusterMemberSa.Name, hostNs, memberNs, memberToolchainClusterName) + verifyToolchainClusterSecret(t, fakeClient, toolchainClusterMemberSa.Name, test.HostOperatorNs, test.MemberOperatorNs, memberToolchainClusterName) // the host secret in the member namespace - verifyToolchainClusterSecret(t, fakeClient, toolchainClusterHostSa.Name, memberNs, hostNs, hostToolchainClusterName) + verifyToolchainClusterSecret(t, fakeClient, toolchainClusterHostSa.Name, test.MemberOperatorNs, test.HostOperatorNs, hostToolchainClusterName) tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.HostOperatorNs))) assert.Len(t, tcs.Items, 1) assert.Equal(t, memberToolchainClusterName, tcs.Items[0].Name) // secret ref in tc matches assert.Equal(t, toolchainClusterMemberSa.Name+"-"+memberToolchainClusterName, tcs.Items[0].Spec.SecretRef.Name) - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.MemberOperatorNs))) assert.Len(t, tcs.Items, 1) assert.Equal(t, hostToolchainClusterName, tcs.Items[0].Name) // secret ref in tc matches @@ -116,7 +113,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterInNamespaceWithReadyCondition(t, fakeClient, memberNs, hostNs, memberNs) // we set to ready only the host toolchaincluster in member operator namespace + mockCreateToolchainClusterInNamespaceWithReadyCondition(t, fakeClient, test.MemberOperatorNs) // we set to ready only the host toolchaincluster in member operator namespace ctx := newExtendedCommandContext(term, newClient) // when @@ -125,9 +122,9 @@ func TestRegisterMember(t *testing.T) { // then require.Error(t, err) tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.HostOperatorNs))) assert.Len(t, tcs.Items, 1) - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.MemberOperatorNs))) assert.Len(t, tcs.Items, 1) assert.Contains(t, term.Output(), "The ToolchainCluster resource representing the member in the host cluster has not become ready.") }) @@ -136,7 +133,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterInNamespaceWithReadyCondition(t, fakeClient, hostNs, hostNs, memberNs) // set to ready only the member toolchaincluster in host operator namespace + mockCreateToolchainClusterInNamespaceWithReadyCondition(t, fakeClient, test.HostOperatorNs) // set to ready only the member toolchaincluster in host operator namespace ctx := newExtendedCommandContext(term, newClient) // when @@ -145,10 +142,10 @@ func TestRegisterMember(t *testing.T) { // then require.Error(t, err) tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.HostOperatorNs))) assert.Empty(t, tcs.Items) assert.Contains(t, term.Output(), "The ToolchainCluster resource representing the host in the member cluster has not become ready.") - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.MemberOperatorNs))) assert.Len(t, tcs.Items, 1) }) @@ -156,7 +153,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) ctx := newExtendedCommandContext(term, newClient) // when @@ -172,7 +169,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) ctx := newExtendedCommandContext(term, newClient) // when @@ -192,7 +189,7 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "member-cool-server.com", - Namespace: hostNs, + Namespace: test.HostOperatorNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ APIEndpoint: "https://cool-server.com", @@ -208,7 +205,7 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster.Name = "member-cool-server.com1" require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster.DeepCopy())) - mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) // when err := registerMemberCluster(ctx, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "2")) @@ -227,7 +224,7 @@ func TestRegisterMember(t *testing.T) { term1 := NewFakeTerminalWithResponse("Y") term2 := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) ctx1 := newExtendedCommandContext(term1, newClient) ctx2 := newExtendedCommandContext(term2, newClient) @@ -250,7 +247,7 @@ func TestRegisterMember(t *testing.T) { term1 := NewFakeTerminalWithResponse("Y") term2 := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) ctx1 := newExtendedCommandContext(term1, newClient) ctx2 := newExtendedCommandContext(term2, newClient) @@ -274,12 +271,12 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) - mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) ctx := newExtendedCommandContext(term, newClient) preexistingToolchainCluster1 := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "host-not-so-cool-server.com", - Namespace: memberNs, + Namespace: test.MemberOperatorNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ APIEndpoint: "https://not-so-cool-server.com", @@ -294,7 +291,7 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster2 := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "host-uncool-server.com", - Namespace: memberNs, + Namespace: test.MemberOperatorNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ APIEndpoint: "https://uncool-server.com", @@ -327,7 +324,7 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "host-not-so-cool-server.com", - Namespace: memberNs, + Namespace: test.MemberOperatorNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ APIEndpoint: "https://not-so-cool-server.com", @@ -359,7 +356,7 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "host-with-weird-name", - Namespace: memberNs, + Namespace: test.MemberOperatorNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ APIEndpoint: "https://cool-server.com", @@ -391,11 +388,11 @@ func TestRegisterMember(t *testing.T) { preexistingToolchainCluster := &toolchainv1alpha1.ToolchainCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "member-with-weird-name", - Namespace: hostNs, + Namespace: test.HostOperatorNs, }, Status: toolchainv1alpha1.ToolchainClusterStatus{ APIEndpoint: "https://cool-server.com", - OperatorNamespace: memberNs, + OperatorNamespace: test.MemberOperatorNs, Conditions: []toolchainv1alpha1.Condition{ { Type: toolchainv1alpha1.ConditionReady, @@ -420,7 +417,7 @@ func TestRegisterMember(t *testing.T) { // given term := NewFakeTerminalWithResponse("Y") newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterHostSa) // we pre-provision only the host toolchaincluster ServiceAccount - mockCreateToolchainClusterWithReadyCondition(t, fakeClient, hostNs, memberNs) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) ctx := newExtendedCommandContext(term, newClient) // when @@ -430,9 +427,9 @@ func TestRegisterMember(t *testing.T) { require.Error(t, err) assert.Contains(t, term.Output(), "The toolchain-member-operator/toolchaincluster-member ServiceAccount is not present in the member cluster.") tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.HostOperatorNs))) assert.Empty(t, tcs.Items) - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.MemberOperatorNs))) assert.Len(t, tcs.Items, 1) }) @@ -449,41 +446,43 @@ func TestRegisterMember(t *testing.T) { require.Error(t, err) assert.Contains(t, term.Output(), "The toolchain-host-operator/toolchaincluster-host ServiceAccount is not present in the host cluster.") tcs := &toolchainv1alpha1.ToolchainClusterList{} - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(hostNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.HostOperatorNs))) assert.Empty(t, tcs.Items) - require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(memberNs))) + require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.MemberOperatorNs))) assert.Empty(t, tcs.Items) }) } -func mockCreateToolchainClusterInNamespaceWithReadyCondition(t *testing.T, fakeClient *test.FakeClient, namespace string, hostOperatorNamespace, memberOperatorNamespace string) { +func mockCreateToolchainClusterInNamespaceWithReadyCondition(t *testing.T, fakeClient *test.FakeClient, namespace string) { fakeClient.MockCreate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.CreateOption) error { if obj, ok := obj.(*toolchainv1alpha1.ToolchainCluster); ok { if obj.GetNamespace() == namespace { - _mockCreateToolchainClusterWithReadyConditionModify(t, obj, hostOperatorNamespace, memberOperatorNamespace) + fillStatusWithDetailsAndReadyCondition(t, obj) } } return fakeClient.Client.Create(ctx, obj, opts...) } } -func mockCreateToolchainClusterWithReadyCondition(t *testing.T, fakeClient *test.FakeClient, hostOperatorNamespace, memberOperatorNamespace string) { +func mockCreateToolchainClusterWithReadyCondition(t *testing.T, fakeClient *test.FakeClient) { fakeClient.MockCreate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.CreateOption) error { if obj, ok := obj.(*toolchainv1alpha1.ToolchainCluster); ok { - _mockCreateToolchainClusterWithReadyConditionModify(t, obj, hostOperatorNamespace, memberOperatorNamespace) + fillStatusWithDetailsAndReadyCondition(t, obj) } return fakeClient.Client.Create(ctx, obj, opts...) } } -func _mockCreateToolchainClusterWithReadyConditionModify(t *testing.T, obj *toolchainv1alpha1.ToolchainCluster, hostOperatorNamespace, memberOperatorNamespace string) { +func fillStatusWithDetailsAndReadyCondition(t *testing.T, obj *toolchainv1alpha1.ToolchainCluster) { var operatorNamespace string - if obj.GetNamespace() == hostOperatorNamespace { - operatorNamespace = memberOperatorNamespace - } else if obj.GetNamespace() == memberOperatorNamespace { - operatorNamespace = hostOperatorNamespace - } else { - assert.Fail(t, "the mock create of ToolchainCluster only works in host operator namespace %s or member operator namespace %s but the creation of toolchain cluster was requested in %s", hostOperatorNamespace, memberOperatorNamespace, obj.GetNamespace()) + switch obj.GetNamespace() { + case test.HostOperatorNs: + operatorNamespace = test.MemberOperatorNs + case test.MemberOperatorNs: + operatorNamespace = test.HostOperatorNs + default: + // If we get here, there is a logic error in the test. Let's fail the test unequivocally. + assert.Fail(t, "the mock create of ToolchainCluster only works in host operator namespace %s or member operator namespace %s but the creation of toolchain cluster was requested in %s", test.HostOperatorNs, test.MemberOperatorNs, obj.GetNamespace()) } obj.Status = toolchainv1alpha1.ToolchainClusterStatus{ APIEndpoint: "https://cool-server.com",