From 6e8e4e1870cbf38d1cdc6f4b40179dbdc2dab6c3 Mon Sep 17 00:00:00 2001 From: Srinivas Atmakuri Date: Mon, 5 Feb 2024 07:15:28 +0530 Subject: [PATCH] HiveNamespace to use docID/clusterID for new Installs (#2992) HiveNamespace currently uses aro-, this change is an effort to unify UUIDs accross cluster doc instead of having multiple, by pointing HiveNamespace to docID so this can be leveraged later. More Details: https://redhat-internal.slack.com/archives/C02ULBRS68M/p1686806655273309 --- pkg/cluster/adminupdate_test.go | 2 +- pkg/cluster/hive.go | 2 +- pkg/cluster/hive_test.go | 25 +++++++++++++------------ pkg/hive/manager.go | 7 +++---- pkg/hive/manager_test.go | 11 ++++++----- pkg/util/mocks/hive/hive.go | 8 ++++---- test/e2e/hive_manager.go | 3 ++- 7 files changed, 30 insertions(+), 28 deletions(-) diff --git a/pkg/cluster/adminupdate_test.go b/pkg/cluster/adminupdate_test.go index 0fb35d1f29d..2a3473ba3ab 100644 --- a/pkg/cluster/adminupdate_test.go +++ b/pkg/cluster/adminupdate_test.go @@ -306,7 +306,7 @@ func TestAdminUpdateSteps(t *testing.T) { doc := baseClusterDoc() doc.OpenShiftCluster.Properties.ProvisioningState = api.ProvisioningStateAdminUpdating doc.OpenShiftCluster.Properties.MaintenanceTask = api.MaintenanceTaskEverything - doc.OpenShiftCluster.Properties.HiveProfile.Namespace = "some_namespace" + doc.OpenShiftCluster.Properties.HiveProfile.Namespace = "aro-00000000-0000-0000-0000-000000000000" doc.OpenShiftCluster.Properties.HiveProfile.CreatedByHive = true return doc, true }, diff --git a/pkg/cluster/hive.go b/pkg/cluster/hive.go index 477081bc812..c19a2e09792 100644 --- a/pkg/cluster/hive.go +++ b/pkg/cluster/hive.go @@ -17,7 +17,7 @@ func (m *manager) hiveCreateNamespace(ctx context.Context) error { return nil } - namespace, err := m.hiveClusterManager.CreateNamespace(ctx) + namespace, err := m.hiveClusterManager.CreateNamespace(ctx, m.doc.ID) if err != nil { return err } diff --git a/pkg/cluster/hive_test.go b/pkg/cluster/hive_test.go index fa46200bc6f..5f7021f726e 100644 --- a/pkg/cluster/hive_test.go +++ b/pkg/cluster/hive_test.go @@ -21,8 +21,7 @@ import ( ) func TestHiveClusterDeploymentReady(t *testing.T) { - fakeNamespace := "fake-namespace" - + fakeNamespace := "aro-00000000-0000-0000-0000-000000000000" for _, tt := range []struct { name string mocks func(hiveMock *mock_hive.MockClusterManager, doc *api.OpenShiftClusterDocument) @@ -75,7 +74,7 @@ func TestHiveClusterDeploymentReady(t *testing.T) { } func TestHiveResetCorrelationData(t *testing.T) { - fakeNamespace := "fake-namespace" + fakeNamespace := "aro-00000000-0000-0000-0000-000000000000" for _, tt := range []struct { name string @@ -115,6 +114,8 @@ func TestHiveResetCorrelationData(t *testing.T) { } func TestHiveCreateNamespace(t *testing.T) { + fakeNamespace := "aro-00000000-0000-0000-0000-000000000000" + fakeNewNamespace := "aro-11111111-1111-1111-1111-111111111111" for _, tt := range []struct { testName string existingNamespaceName string @@ -126,36 +127,36 @@ func TestHiveCreateNamespace(t *testing.T) { { testName: "creates namespace if it doesn't exist", existingNamespaceName: "", - newNamespaceName: "new-namespace", + newNamespaceName: fakeNamespace, clusterManagerMock: func(mockCtrl *gomock.Controller, namespaceName string) *mock_hive.MockClusterManager { namespaceToReturn := &corev1.Namespace{} namespaceToReturn.Name = namespaceName mockClusterManager := mock_hive.NewMockClusterManager(mockCtrl) - mockClusterManager.EXPECT().CreateNamespace(gomock.Any()).Return(namespaceToReturn, nil) + mockClusterManager.EXPECT().CreateNamespace(gomock.Any(), gomock.Any()).Return(namespaceToReturn, nil) return mockClusterManager }, - expectedNamespaceName: "new-namespace", + expectedNamespaceName: fakeNamespace, wantErr: "", }, { testName: "doesn't create namespace if it already exists", - existingNamespaceName: "existing-namespace", - newNamespaceName: "new-namespace", - expectedNamespaceName: "existing-namespace", + existingNamespaceName: fakeNamespace, + newNamespaceName: fakeNewNamespace, + expectedNamespaceName: fakeNamespace, clusterManagerMock: func(mockCtrl *gomock.Controller, namespaceName string) *mock_hive.MockClusterManager { mockClusterManager := mock_hive.NewMockClusterManager(mockCtrl) - mockClusterManager.EXPECT().CreateNamespace(gomock.Any()).Times(0) + mockClusterManager.EXPECT().CreateNamespace(gomock.Any(), gomock.Any()).Times(0) return mockClusterManager }, }, { testName: "returns error if cluster manager returns error", existingNamespaceName: "", - newNamespaceName: "new-namespace", + newNamespaceName: fakeNamespace, expectedNamespaceName: "", clusterManagerMock: func(mockCtrl *gomock.Controller, namespaceName string) *mock_hive.MockClusterManager { mockClusterManager := mock_hive.NewMockClusterManager(mockCtrl) - mockClusterManager.EXPECT().CreateNamespace(gomock.Any()).Return(nil, fmt.Errorf("cluster manager error")) + mockClusterManager.EXPECT().CreateNamespace(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("cluster manager error")) return mockClusterManager }, wantErr: "cluster manager error", diff --git a/pkg/hive/manager.go b/pkg/hive/manager.go index f12c425be5c..17ba3623ed4 100644 --- a/pkg/hive/manager.go +++ b/pkg/hive/manager.go @@ -24,11 +24,10 @@ import ( "github.com/Azure/ARO-RP/pkg/hive/failure" "github.com/Azure/ARO-RP/pkg/util/dynamichelper" utillog "github.com/Azure/ARO-RP/pkg/util/log" - "github.com/Azure/ARO-RP/pkg/util/uuid" ) type ClusterManager interface { - CreateNamespace(ctx context.Context) (*corev1.Namespace, error) + CreateNamespace(ctx context.Context, docID string) (*corev1.Namespace, error) // CreateOrUpdate reconciles the ClusterDocument and related secrets for an // existing cluster. This may adopt the cluster (Create) or amend the @@ -110,11 +109,11 @@ func NewFromConfig(log *logrus.Entry, _env env.Core, restConfig *rest.Config) (C }, nil } -func (hr *clusterManager) CreateNamespace(ctx context.Context) (*corev1.Namespace, error) { +func (hr *clusterManager) CreateNamespace(ctx context.Context, docID string) (*corev1.Namespace, error) { var namespaceName string var namespace *corev1.Namespace err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - namespaceName = "aro-" + uuid.DefaultGenerator.Generate() + namespaceName = "aro-" + docID namespace = &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: namespaceName, diff --git a/pkg/hive/manager_test.go b/pkg/hive/manager_test.go index 9f3836cd2c0..a4136a2c1a6 100644 --- a/pkg/hive/manager_test.go +++ b/pkg/hive/manager_test.go @@ -26,7 +26,7 @@ import ( ) func TestIsClusterDeploymentReady(t *testing.T) { - fakeNamespace := "fake-namespace" + fakeNamespace := "aro-00000000-0000-0000-0000-000000000000" doc := &api.OpenShiftClusterDocument{ OpenShiftCluster: &api.OpenShiftCluster{ Properties: api.OpenShiftClusterProperties{ @@ -170,7 +170,7 @@ func TestIsClusterDeploymentReady(t *testing.T) { } func TestIsClusterInstallationComplete(t *testing.T) { - fakeNamespace := "fake-namespace" + fakeNamespace := "aro-00000000-0000-0000-0000-000000000000" doc := &api.OpenShiftClusterDocument{ OpenShiftCluster: &api.OpenShiftCluster{ Properties: api.OpenShiftClusterProperties{ @@ -413,7 +413,7 @@ func TestIsClusterInstallationComplete(t *testing.T) { } func TestResetCorrelationData(t *testing.T) { - fakeNamespace := "fake-namespace" + fakeNamespace := "aro-00000000-0000-0000-0000-000000000000" doc := &api.OpenShiftClusterDocument{ OpenShiftCluster: &api.OpenShiftCluster{ Properties: api.OpenShiftClusterProperties{ @@ -478,6 +478,7 @@ func TestResetCorrelationData(t *testing.T) { } func TestCreateNamespace(t *testing.T) { + const docID = "00000000-0000-0000-0000-000000000000" for _, tc := range []struct { name string nsNames []string @@ -519,7 +520,7 @@ func TestCreateNamespace(t *testing.T) { uuid.DefaultGenerator = uuidfake.NewGenerator(tc.nsNames) } - ns, err := c.CreateNamespace(context.Background()) + ns, err := c.CreateNamespace(context.Background(), docID) if err != nil && !tc.shouldFail { t.Error(err) } @@ -538,7 +539,7 @@ func TestCreateNamespace(t *testing.T) { } func TestGetClusterDeployment(t *testing.T) { - fakeNamespace := "fake-namespace" + fakeNamespace := "aro-00000000-0000-0000-0000-000000000000" doc := &api.OpenShiftClusterDocument{ OpenShiftCluster: &api.OpenShiftCluster{ Properties: api.OpenShiftClusterProperties{ diff --git a/pkg/util/mocks/hive/hive.go b/pkg/util/mocks/hive/hive.go index a5a9408bb81..dd6e22e2715 100644 --- a/pkg/util/mocks/hive/hive.go +++ b/pkg/util/mocks/hive/hive.go @@ -39,18 +39,18 @@ func (m *MockClusterManager) EXPECT() *MockClusterManagerMockRecorder { } // CreateNamespace mocks base method. -func (m *MockClusterManager) CreateNamespace(arg0 context.Context) (*v10.Namespace, error) { +func (m *MockClusterManager) CreateNamespace(arg0 context.Context, arg1 string) (*v10.Namespace, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateNamespace", arg0) + ret := m.ctrl.Call(m, "CreateNamespace", arg0, arg1) ret0, _ := ret[0].(*v10.Namespace) ret1, _ := ret[1].(error) return ret0, ret1 } // CreateNamespace indicates an expected call of CreateNamespace. -func (mr *MockClusterManagerMockRecorder) CreateNamespace(arg0 interface{}) *gomock.Call { +func (mr *MockClusterManagerMockRecorder) CreateNamespace(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateNamespace", reflect.TypeOf((*MockClusterManager)(nil).CreateNamespace), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateNamespace", reflect.TypeOf((*MockClusterManager)(nil).CreateNamespace), arg0, arg1) } // CreateOrUpdate mocks base method. diff --git a/test/e2e/hive_manager.go b/test/e2e/hive_manager.go index 9a77ca57ab5..953c6e15eb4 100644 --- a/test/e2e/hive_manager.go +++ b/test/e2e/hive_manager.go @@ -28,8 +28,9 @@ var _ = Describe("Hive manager creates a namespace", func() { }) It("Should be created successfully", func() { + const docID = "00000000-0000-0000-0000-000000000000" var err error - ns, err = clients.HiveClusterManager.CreateNamespace(context.Background()) + ns, err = clients.HiveClusterManager.CreateNamespace(context.Background(), docID) Expect(err).NotTo(HaveOccurred()) Expect(ns).NotTo(BeNil())