diff --git a/charts/member-agent-arc/templates/deployment.yaml b/charts/member-agent-arc/templates/deployment.yaml index 195dc29ba..30f8ef5ff 100644 --- a/charts/member-agent-arc/templates/deployment.yaml +++ b/charts/member-agent-arc/templates/deployment.yaml @@ -21,7 +21,7 @@ spec: imagePullPolicy: IfNotPresent image: "{{ .Values.crdinstaller.repository }}:{{ .Values.crdinstaller.tag }}" args: - - --mode=member + - --mode=arcMember - --v={{ .Values.crdinstaller.logVerbosity }} securityContext: capabilities: diff --git a/cmd/crdinstaller/main.go b/cmd/crdinstaller/main.go index ba945f981..1f6639fb4 100644 --- a/cmd/crdinstaller/main.go +++ b/cmd/crdinstaller/main.go @@ -22,15 +22,17 @@ import ( "go.goms.io/fleet/cmd/crdinstaller/utils" ) -var mode = flag.String("mode", "", "Mode to run in: 'hub' or 'member' (required)") +var ( + mode = flag.String("mode", "", "Mode to run in: 'hub', 'member', 'arcMember', (required)") +) func main() { klog.InitFlags(nil) flag.Parse() // Validate required flags. - if *mode != "hub" && *mode != "member" { - klog.Fatal("--mode flag must be either 'hub' or 'member'") + if *mode != utils.ModeHub && *mode != utils.ModeMember && *mode != utils.ModeArcMember { + klog.Fatal("--mode flag must be either 'hub' or 'member' or 'arcMember'") } klog.Infof("Starting CRD installer in %s mode", *mode) @@ -89,7 +91,7 @@ func installCRDs(ctx context.Context, client client.Client, crdPath, mode string // Install each CRD. for i := range crdsToInstall { - if err := utils.InstallCRD(ctx, client, &crdsToInstall[i]); err != nil { + if err := utils.InstallCRD(ctx, client, &crdsToInstall[i], mode); err != nil { return err } } diff --git a/cmd/crdinstaller/utils/util.go b/cmd/crdinstaller/utils/util.go index 2ff94dc22..6601c59ad 100644 --- a/cmd/crdinstaller/utils/util.go +++ b/cmd/crdinstaller/utils/util.go @@ -24,6 +24,8 @@ import ( ) const ( + // ArcInstallationKey is the key used to indicate if the installation is for ARC AKS cluster. + ArcInstallationKey = "crd-installer.azurefleet.io/arc" // CRDInstallerLabelKey is the label key used to indicate that a CRD is managed by the installer. CRDInstallerLabelKey = "crd-installer.azurefleet.io/managed" // AzureManagedLabelKey is the label key used to indicate that a CRD is managed by an azure resource. @@ -32,6 +34,20 @@ const ( FleetLabelValue = "fleet" ) +const ( + trueLabelValue = "true" +) + +// Mode constants for CRD installer. +const ( + // ModeHub installs hub cluster CRDs. + ModeHub = "hub" + // ModeMember installs member cluster CRDs. + ModeMember = "member" + // ModeArcMember installs member cluster CRDs with ARC labels. + ModeArcMember = "arcMember" +) + var ( multiclusterCRD = map[string]bool{ "multicluster.x-k8s.io_clusterprofiles.yaml": true, @@ -42,7 +58,7 @@ var ( ) // InstallCRD creates/updates a Custom Resource Definition (CRD) from the provided CRD object. -func InstallCRD(ctx context.Context, client client.Client, crd *apiextensionsv1.CustomResourceDefinition) error { +func InstallCRD(ctx context.Context, client client.Client, crd *apiextensionsv1.CustomResourceDefinition, mode string) error { klog.V(2).Infof("Installing CRD: %s", crd.Name) existingCRD := apiextensionsv1.CustomResourceDefinition{ @@ -59,8 +75,13 @@ func InstallCRD(ctx context.Context, client client.Client, crd *apiextensionsv1. if existingCRD.Labels == nil { existingCRD.Labels = make(map[string]string) } + if mode == ModeArcMember { + // For ARC AKS installation, we want to add an additional label to indicate this is an ARC managed cluster, + // needed for clean up of CRD by kube-addon-manager. + existingCRD.Labels[ArcInstallationKey] = trueLabelValue + } // Ensure the label for management by the installer is set. - existingCRD.Labels[CRDInstallerLabelKey] = "true" + existingCRD.Labels[CRDInstallerLabelKey] = trueLabelValue // Also set the Azure managed label to indicate this is managed by Fleet, // needed for clean up of CRD by kube-addon-manager. existingCRD.Labels[AzureManagedLabelKey] = FleetLabelValue @@ -106,35 +127,22 @@ func CollectCRDs(crdDirectoryPath, mode string, scheme *runtime.Scheme) ([]apiex // Process based on mode. crdFileName := filepath.Base(crdpath) - if mode == "member" { - if memberCRD[crdFileName] { - crd, err := GetCRDFromPath(crdpath, scheme) - if err != nil { - return err - } - crdsToInstall = append(crdsToInstall, *crd) - } - // Skip CRDs that are not in the memberCRD map. - return nil + var shouldInstall bool + switch mode { + case ModeMember, ModeArcMember: + shouldInstall = memberCRD[crdFileName] + case ModeHub: + // Install multicluster CRD or CRDs with kubernetes-fleet.io in the filename (excluding member-only CRDs). + // CRD filenames follow the pattern _.yaml, so we can check the filename. + shouldInstall = multiclusterCRD[crdFileName] || (strings.Contains(crdFileName, "kubernetes-fleet.io") && !memberCRD[crdFileName]) } - crd, err := GetCRDFromPath(crdpath, scheme) - if err != nil { - return err - } - - // For hub mode, only collect CRDs whose group has substring kubernetes-fleet.io. - if mode == "hub" { - // special case for multicluster external CRD in hub cluster. - if multiclusterCRD[crdFileName] { - crdsToInstall = append(crdsToInstall, *crd) - return nil - } - group := crd.Spec.Group - // Check if the group contains "kubernetes-fleet.io" substring. - if strings.Contains(group, "kubernetes-fleet.io") && !memberCRD[crdFileName] { - crdsToInstall = append(crdsToInstall, *crd) + if shouldInstall { + crd, err := GetCRDFromPath(crdpath, scheme) + if err != nil { + return err } + crdsToInstall = append(crdsToInstall, *crd) } return nil diff --git a/cmd/crdinstaller/utils/util_test.go b/cmd/crdinstaller/utils/util_test.go index 22f474b7d..bb67ee577 100644 --- a/cmd/crdinstaller/utils/util_test.go +++ b/cmd/crdinstaller/utils/util_test.go @@ -56,7 +56,7 @@ func runTest(t *testing.T, crdPath string) { }{ { name: "hub mode v1beta1 with actual directory", - mode: "hub", + mode: ModeHub, wantedCRDNames: []string{ "memberclusters.cluster.kubernetes-fleet.io", "internalmemberclusters.cluster.kubernetes-fleet.io", @@ -90,7 +90,7 @@ func runTest(t *testing.T, crdPath string) { }, { name: "member mode v1beta1 with actual directory", - mode: "member", + mode: ModeMember, wantedCRDNames: []string{ "appliedworks.placement.kubernetes-fleet.io", }, @@ -157,11 +157,19 @@ func TestInstallCRD(t *testing.T) { tests := []struct { name string crd *apiextensionsv1.CustomResourceDefinition + mode string wantError bool }{ { - name: "successful CRD installation", + name: "successful CRD installation with member mode", crd: testCRD, + mode: ModeMember, + wantError: false, + }, + { + name: "successful CRD installation with arcMember mode", + crd: testCRD, + mode: ModeArcMember, wantError: false, }, } @@ -169,7 +177,7 @@ func TestInstallCRD(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() - err := InstallCRD(context.Background(), fakeClient, tt.crd) + err := InstallCRD(context.Background(), fakeClient, tt.crd, tt.mode) if tt.wantError { if err == nil { @@ -198,6 +206,16 @@ func TestInstallCRD(t *testing.T) { t.Errorf("Expected CRD label %s to be %q, got %q", AzureManagedLabelKey, FleetLabelValue, installedCRD.Labels[AzureManagedLabelKey]) } + if tt.mode == ModeArcMember { + if installedCRD.Labels[ArcInstallationKey] != "true" { + t.Errorf("Expected CRD label %s to be 'true', got %q", ArcInstallationKey, installedCRD.Labels[ArcInstallationKey]) + } + } else { + if _, exists := installedCRD.Labels[ArcInstallationKey]; exists { + t.Errorf("Expected CRD label %s to not exist for non-ARC installation", ArcInstallationKey) + } + } + if diff := cmp.Diff(tt.crd.Spec, installedCRD.Spec); diff != "" { t.Errorf("CRD spec mismatch (-want +got):\n%s", diff) } diff --git a/test/crdinstaller/crd_installer_integration_test.go b/test/crdinstaller/crd_installer_integration_test.go index 7aee4a5d0..cbc4daebd 100644 --- a/test/crdinstaller/crd_installer_integration_test.go +++ b/test/crdinstaller/crd_installer_integration_test.go @@ -13,9 +13,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" cmdCRDInstaller "go.goms.io/fleet/cmd/crdinstaller/utils" + "go.goms.io/fleet/pkg/utils" ) const ( @@ -33,53 +35,115 @@ const ( // It ensures that the installer can create a CRD, update it with new fields, and handle ownership label correctly. // The original CRD has 4 properties, and the updated CRD has a new property to simulate CRD upgrade. var _ = Describe("Test CRD Installer, Create and Update CRD", Ordered, func() { - It("should create original CRD", func() { - crd, err := cmdCRDInstaller.GetCRDFromPath(originalCRDPath, scheme) - Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", originalCRDPath) - Expect(cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd)).To(Succeed()) + AfterEach(OncePerOrdered, func() { + deleteCRD(crdName) }) - It("should verify original CRD installation", func() { - ensureCRDExistsWithLabels(map[string]string{ - cmdCRDInstaller.CRDInstallerLabelKey: "true", - cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue, + Context("without ARC installation mode", func() { + It("should create original CRD", func() { + crd, err := cmdCRDInstaller.GetCRDFromPath(originalCRDPath, scheme) + Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", originalCRDPath) + Eventually(func() error { + return cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd, cmdCRDInstaller.ModeMember) + }, eventuallyDuration, eventuallyInterval).Should(Succeed()) }) - crd := &apiextensionsv1.CustomResourceDefinition{} - Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName) - spec := fetchSpecJSONSchemaProperties(crd) - // Original CRD should have 4 properties defined in spec. - Expect(len(spec.Properties)).Should(Equal(4), "CRD %s should have 4 properties defined in spec", crdName) - Expect(spec.Properties["bar"].Type).Should(Equal("string"), "CRD %s should have 'bar' property of type string defined in properties", crdName) - _, ok := spec.Properties["newField"] - Expect(ok).To(BeFalse(), "CRD %s should not have 'newField' property defined in properties", crdName) - }) - It("update the CRD to add a random label", func() { - updateCRDLabels(crdName, map[string]string{randomLabelKey: "true"}) - }) + It("should verify original CRD installation", func() { + ensureCRDExistsWithLabels(map[string]string{ + cmdCRDInstaller.CRDInstallerLabelKey: "true", + cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue, + }) + crd := &apiextensionsv1.CustomResourceDefinition{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName) + spec := fetchSpecJSONSchemaProperties(crd) + // Original CRD should have 4 properties defined in spec. + Expect(len(spec.Properties)).Should(Equal(4), "CRD %s should have 4 properties defined in spec", crdName) + Expect(spec.Properties["bar"].Type).Should(Equal("string"), "CRD %s should have 'bar' property of type string defined in properties", crdName) + _, ok := spec.Properties["newField"] + Expect(ok).To(BeFalse(), "CRD %s should not have 'newField' property defined in properties", crdName) + }) + + It("update the CRD to add a random label", func() { + updateCRDLabels(crdName, map[string]string{randomLabelKey: "true"}) + }) - It("should update the CRD with new field in spec with crdinstaller label", func() { - crd, err := cmdCRDInstaller.GetCRDFromPath(updatedCRDPath, scheme) - Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", updatedCRDPath) - Expect(cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd)).To(Succeed()) + It("should update the CRD with new field in spec with crdinstaller label", func() { + crd, err := cmdCRDInstaller.GetCRDFromPath(updatedCRDPath, scheme) + Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", updatedCRDPath) + Eventually(func() error { + return cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd, cmdCRDInstaller.ModeMember) + }, eventuallyDuration, eventuallyInterval).Should(Succeed()) + }) + + It("should verify updated CRD", func() { + // ensure we don't overwrite the random label. + ensureCRDExistsWithLabels(map[string]string{ + randomLabelKey: "true", + cmdCRDInstaller.CRDInstallerLabelKey: "true", + cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue, + }) + crd := &apiextensionsv1.CustomResourceDefinition{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName) + spec := fetchSpecJSONSchemaProperties(crd) + // Updated CRD should have 5 properties defined in spec. + Expect(len(spec.Properties)).Should(Equal(5), "CRD %s should have 5 properties defined in spec", crdName) + Expect(spec.Properties["bar"].Type).Should(Equal("string"), "CRD %s should have 'bar' property of type string defined in properties", crdName) + _, ok := spec.Properties["newField"] + Expect(ok).To(BeTrue(), "CRD %s should have 'newField' property defined in properties", crdName) + Expect(spec.Properties["newField"].Type).Should(Equal("string"), "CRD %s should have 'newField' property of type string defined in properties", crdName) + }) }) - It("should verify updated CRD", func() { - // ensure we don't overwrite the random label. - ensureCRDExistsWithLabels(map[string]string{ - randomLabelKey: "true", - cmdCRDInstaller.CRDInstallerLabelKey: "true", - cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue, + Context("with ARC installation mode", func() { + It("should create CRD with ARC installation mode", func() { + crd, err := cmdCRDInstaller.GetCRDFromPath(originalCRDPath, scheme) + Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", originalCRDPath) + Eventually(func() error { + return cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd, cmdCRDInstaller.ModeArcMember) + }, eventuallyDuration, eventuallyInterval).Should(Succeed()) + }) + + It("should verify CRD has ARC installation label", func() { + ensureCRDExistsWithLabels(map[string]string{ + cmdCRDInstaller.CRDInstallerLabelKey: "true", + cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue, + cmdCRDInstaller.ArcInstallationKey: "true", + }) + crd := &apiextensionsv1.CustomResourceDefinition{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName) + spec := fetchSpecJSONSchemaProperties(crd) + // Original CRD should have 4 properties defined in spec. + Expect(len(spec.Properties)).Should(Equal(4), "CRD %s should have 4 properties defined in spec", crdName) + }) + + It("update the CRD to add a random label", func() { + updateCRDLabels(crdName, map[string]string{randomLabelKey: "true"}) + }) + + It("should update the CRD with new field in spec while preserving ARC label", func() { + crd, err := cmdCRDInstaller.GetCRDFromPath(updatedCRDPath, scheme) + Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", updatedCRDPath) + Eventually(func() error { + return cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd, cmdCRDInstaller.ModeArcMember) + }, eventuallyDuration, eventuallyInterval).Should(Succeed()) + }) + + It("should verify updated CRD has all labels including ARC", func() { + // ensure we don't overwrite the random label and ARC label is preserved. + ensureCRDExistsWithLabels(map[string]string{ + randomLabelKey: "true", + cmdCRDInstaller.CRDInstallerLabelKey: "true", + cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue, + cmdCRDInstaller.ArcInstallationKey: "true", + }) + crd := &apiextensionsv1.CustomResourceDefinition{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName) + spec := fetchSpecJSONSchemaProperties(crd) + // Updated CRD should have 5 properties defined in spec. + Expect(len(spec.Properties)).Should(Equal(5), "CRD %s should have 5 properties defined in spec", crdName) + _, ok := spec.Properties["newField"] + Expect(ok).To(BeTrue(), "CRD %s should have 'newField' property defined in properties", crdName) }) - crd := &apiextensionsv1.CustomResourceDefinition{} - Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName) - spec := fetchSpecJSONSchemaProperties(crd) - // Updated CRD should have 5 properties defined in spec. - Expect(len(spec.Properties)).Should(Equal(5), "CRD %s should have 5 properties defined in spec", crdName) - Expect(spec.Properties["bar"].Type).Should(Equal("string"), "CRD %s should have 'bar' property of type string defined in properties", crdName) - _, ok := spec.Properties["newField"] - Expect(ok).To(BeTrue(), "CRD %s should have 'newField' property defined in properties", crdName) - Expect(spec.Properties["newField"].Type).Should(Equal("string"), "CRD %s should have 'newField' property of type string defined in properties", crdName) }) }) @@ -115,5 +179,17 @@ func ensureCRDExistsWithLabels(wantLabels map[string]string) { return fmt.Errorf("crd labels mismatch (-want, +got) :\n%s", diff) } return nil - }, eventuallyDuration, eventuallyInterval).ShouldNot(HaveOccurred(), "CRD %s should exist with labels %v", crdName) + }, eventuallyDuration, eventuallyInterval).ShouldNot(HaveOccurred(), "CRD %s should exist with labels %v", crdName, wantLabels) +} + +func deleteCRD(crdName string) { + crd := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: crdName, + }, + } + Expect(k8sClient.Delete(ctx, crd)).Should(SatisfyAny(Succeed(), utils.NotFoundMatcher{})) + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd) + }, eventuallyDuration, eventuallyInterval).Should(utils.NotFoundMatcher{}, "CRD %s should be deleted", crdName) }