Skip to content

feat: add flag on member-agent-arc chart to add new label for CRDs #1253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Arvindthiru merged 12 commits into from
Feb 11, 2026
Merged

feat: add flag on member-agent-arc chart to add new label for CRDs #1253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ea66ee6
add ARC flag, update UTs
Feb 5, 2026
e0788f1
fix IT
Feb 5, 2026
4c80425
update IT
Feb 5, 2026
e0e00e1
add flag to member-agent-arc chart
Feb 5, 2026
e3f9a93
make reviewable
Feb 5, 2026
e908f42
minor IT refactor
Feb 5, 2026
10d85c4
address comment
Feb 5, 2026
1f4b837
address comment
Feb 6, 2026
e8c9b39
minor refactor
Feb 6, 2026
f9306e0
minor refactor
Feb 6, 2026
6c15065
lint fix
Feb 6, 2026
e85d9ac
minor refactor
Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/member-agent-arc/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 6 additions & 4 deletions cmd/crdinstaller/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'")
}
Copy link
Contributor

@Ealianis Ealianis Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: might read better to to have valid modes as a private var and helper func.
e.g.,

var validModes = []string{
	utils.ModeHub,
	utils.ModeMember,
	utils.ModeArcMember,
}

func isValidMode(m string) bool {
	for _, v := range validModes {
		if v == m {
			return true
		}
	}
	return false
}

if !isValidMode(*mode) {
	klog.Fatalf("--mode must be one of: %s", strings.Join(validModes, ", "))
}


klog.Infof("Starting CRD installer in %s mode", *mode)
Expand Down Expand Up @@ -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
}
}
Expand Down
65 changes: 36 additions & 29 deletions cmd/crdinstaller/utils/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,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 member label value.
ModeArcMember = "arcMember"
Copy link
Contributor

@ryanzhang-oss ryanzhang-oss Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this would be called aksAsArcMember?

Copy link
Contributor

@ryanzhang-oss ryanzhang-oss Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or we want to treat them the same?

Copy link
Contributor

@Ealianis Ealianis Feb 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I don't think there is a reason (as of now) to differenetiate here.
Our (AKS) script just needs to know it's an ArcMember; if the cluster is not a MC, then it's just an annotation on a CRD in unmanaged space, i.e., no-op.

Copy link
Contributor

@ryanzhang-oss ryanzhang-oss Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason I ask is if we use arcMember now and need to differentiate later for askAsArc, the code here needs to change.

Copy link
Contributor

@ryanzhang-oss ryanzhang-oss Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given that we might achieve the entire crdInstaller, I guess this is not an issue anymore @Arvindthiru ?

)

var (
multiclusterCRD = map[string]bool{
"multicluster.x-k8s.io_clusterprofiles.yaml": true,
Expand All @@ -42,7 +56,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{
Expand All @@ -60,10 +74,16 @@ func InstallCRD(ctx context.Context, client client.Client, crd *apiextensionsv1.
existingCRD.Labels = make(map[string]string)
}
// 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
if mode == ModeArcMember {
// For aks ARC, we set the label value to "arcMember" to avoid clean up of CRDs by OM.
existingCRD.Labels[AzureManagedLabelKey] = ModeArcMember
} else {
existingCRD.Labels[AzureManagedLabelKey] = FleetLabelValue
}

return nil
}
err := install(ctx, client, &existingCRD, mutFn)
Expand Down Expand Up @@ -106,35 +126,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 <group>_<plural>.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
Expand Down
26 changes: 20 additions & 6 deletions cmd/crdinstaller/utils/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
},
Expand Down Expand Up @@ -157,19 +157,27 @@ 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,
},
}

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 {
Expand All @@ -194,8 +202,14 @@ func TestInstallCRD(t *testing.T) {
t.Errorf("Expected CRD label %s to be 'true', got %q", CRDInstallerLabelKey, installedCRD.Labels[CRDInstallerLabelKey])
}

if installedCRD.Labels[AzureManagedLabelKey] != FleetLabelValue {
t.Errorf("Expected CRD label %s to be %q, got %q", AzureManagedLabelKey, FleetLabelValue, installedCRD.Labels[AzureManagedLabelKey])
if tt.mode == ModeArcMember {
if installedCRD.Labels[AzureManagedLabelKey] != ModeArcMember {
t.Errorf("Expected CRD label %s to be %q, got %q", AzureManagedLabelKey, ModeArcMember, installedCRD.Labels[AzureManagedLabelKey])
}
} else {
if installedCRD.Labels[AzureManagedLabelKey] != FleetLabelValue {
t.Errorf("Expected CRD label %s to be %q, got %q", AzureManagedLabelKey, FleetLabelValue, installedCRD.Labels[AzureManagedLabelKey])
}
}

if diff := cmp.Diff(tt.crd.Spec, installedCRD.Spec); diff != "" {
Expand Down
154 changes: 114 additions & 40 deletions test/crdinstaller/crd_installer_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -33,53 +35,113 @@ 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.ModeArcMember,
})
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.ModeArcMember,
})
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)
})
})

Expand Down Expand Up @@ -115,5 +177,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)
}
Loading