From 403e6ef6f52258ec0db2df19364a5030725c66eb Mon Sep 17 00:00:00 2001 From: Artur Shad Nik Date: Thu, 21 Aug 2025 15:14:18 -0700 Subject: [PATCH 1/3] feat: allow installing and uninstalling hub-addons Signed-off-by: Artur Shad Nik --- .../api/v1alpha1/fleetconfig_types.go | 17 +- .../api/v1alpha1/validation.go | 30 ++- .../api/v1alpha1/zz_generated.deepcopy.go | 20 ++ .../charts/fleetconfig-controller/README.md | 1 + ...onfig.open-cluster-management.io-crds.yaml | 21 ++ .../templates/fleetconfig.yaml | 1 + .../templates/rbac/manager-rbac.yaml | 2 +- .../charts/fleetconfig-controller/values.yaml | 5 + ...en-cluster-management.io_fleetconfigs.yaml | 21 ++ .../internal/controller/addon.go | 187 ++++++++++++++++++ .../internal/controller/hub.go | 10 +- 11 files changed, 307 insertions(+), 8 deletions(-) diff --git a/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go b/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go index 3f0c265c..4e6a010d 100644 --- a/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go +++ b/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go @@ -61,9 +61,10 @@ type FleetConfigSpec struct { // FleetConfigStatus defines the observed state of FleetConfig. type FleetConfigStatus struct { - Phase string `json:"phase,omitempty"` - Conditions []Condition `json:"conditions,omitempty"` - JoinedSpokes []JoinedSpoke `json:"joinedSpokes,omitempty"` + Phase string `json:"phase,omitempty"` + Conditions []Condition `json:"conditions,omitempty"` + JoinedSpokes []JoinedSpoke `json:"joinedSpokes,omitempty"` + InstalledHubAddOns []InstalledHubAddOn `json:"installedHubAddOns,omitempty"` } // ToComparable returns a deep copy of the FleetConfigStatus that's suitable for semantic comparison. @@ -488,6 +489,16 @@ func (j *JoinedSpoke) conditionName(c int) string { return name } +// InstalledHubAddOn tracks metadata for each hubAddon that is successfully installed on the hub. +type InstalledHubAddOn struct { + // BundleVersion is the bundle version used when installing the addon. + BundleVerion string `json:"bundleVersion"` + // Name is the name of the addon. + Name string `json:"name"` + // Namespace is the namespace that the addon was installed into. + Namespace string `json:"namespace,omitempty"` +} + // Klusterlet is the configuration for a klusterlet. type Klusterlet struct { // Annotations to apply to the spoke cluster. If not present, the 'agent.open-cluster-management.io/' prefix is added to each key. diff --git a/fleetconfig-controller/api/v1alpha1/validation.go b/fleetconfig-controller/api/v1alpha1/validation.go index bb04e315..d3b0c3e7 100644 --- a/fleetconfig-controller/api/v1alpha1/validation.go +++ b/fleetconfig-controller/api/v1alpha1/validation.go @@ -84,6 +84,19 @@ func allowFleetConfigUpdate(newObject *FleetConfig, oldObject *FleetConfig) erro // checks that each addOnConfig specifies a valid source of manifests func validateAddonConfigs(ctx context.Context, client client.Client, oldObject, newObject *FleetConfig) field.ErrorList { errs := field.ErrorList{} + + // Validate that AddOnConfig names are unique within the AddOnConfigs list + addOnConfigNames := make(map[string]int) + for i, a := range newObject.Spec.AddOnConfigs { + key := fmt.Sprintf("%s-%s", a.Name, a.Version) + if existingIndex, found := addOnConfigNames[key]; found { + errs = append(errs, field.Invalid(field.NewPath("addOnConfigs").Index(i), key, + fmt.Sprintf("duplicate addOnConfig %s (name-version) found at indices %d and %d", key, existingIndex, i))) + } else { + addOnConfigNames[key] = i + } + } + for i, a := range newObject.Spec.AddOnConfigs { cm := corev1.ConfigMap{} cmName := fmt.Sprintf("%s-%s-%s", AddonConfigMapNamePrefix, a.Name, a.Version) @@ -172,7 +185,7 @@ func validateAddons(newObject *FleetConfig) field.ErrorList { for i, s := range newObject.Spec.Spokes { for j, a := range s.AddOns { if !configuredAddons[a.ConfigName] { - errs = append(errs, field.Invalid(field.NewPath("Spokes").Index(i).Child("AddOns").Index(j), a.ConfigName, fmt.Sprintf("cannot enable addon %s for spoke %s, no configuration found in spec.AddOnConfigs", a.ConfigName, s.Name))) + errs = append(errs, field.Invalid(field.NewPath("Spokes").Index(i).Child("AddOns").Index(j), a.ConfigName, fmt.Sprintf("cannot enable addon %s for spoke %s, no configuration found in spec.AddOnConfigs or spec.HubAddOns", a.ConfigName, s.Name))) } } } @@ -192,8 +205,19 @@ func validateHubAddons(ctx context.Context, oldObject, newObject *FleetConfig) f for i, ha := range newObject.Spec.HubAddOns { if _, found := addOnConfigNames[ha.Name]; found { - errs = append(errs, field.Invalid(field.NewPath("hubAddOn").Index(i), ha.Name, - fmt.Sprintf("hubAddOn name %s clashes with an existing addOnConfig name", ha.Name))) + errs = append(errs, field.Invalid(field.NewPath("hubAddOns").Index(i), ha.Name, + fmt.Sprintf("hubAddOn name %s clashes with an existing addOnConfig name.", ha.Name))) + } + } + + // Validate that HubAddOn names are unique within the HubAddOns list + hubAddOnNames := make(map[string]int) + for i, ha := range newObject.Spec.HubAddOns { + if existingIndex, found := hubAddOnNames[ha.Name]; found { + errs = append(errs, field.Invalid(field.NewPath("hubAddOns").Index(i), ha.Name, + fmt.Sprintf("duplicate hubAddOn name %s found at indices %d and %d", ha.Name, existingIndex, i))) + } else { + hubAddOnNames[ha.Name] = i } } diff --git a/fleetconfig-controller/api/v1alpha1/zz_generated.deepcopy.go b/fleetconfig-controller/api/v1alpha1/zz_generated.deepcopy.go index b8fc6b70..efc18e7d 100644 --- a/fleetconfig-controller/api/v1alpha1/zz_generated.deepcopy.go +++ b/fleetconfig-controller/api/v1alpha1/zz_generated.deepcopy.go @@ -219,6 +219,11 @@ func (in *FleetConfigStatus) DeepCopyInto(out *FleetConfigStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.InstalledHubAddOns != nil { + in, out := &in.InstalledHubAddOns, &out.InstalledHubAddOns + *out = make([]InstalledHubAddOn, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FleetConfigStatus. @@ -307,6 +312,21 @@ func (in *HubAddOn) DeepCopy() *HubAddOn { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *InstalledHubAddOn) DeepCopyInto(out *InstalledHubAddOn) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InstalledHubAddOn. +func (in *InstalledHubAddOn) DeepCopy() *InstalledHubAddOn { + if in == nil { + return nil + } + out := new(InstalledHubAddOn) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JoinedSpoke) DeepCopyInto(out *JoinedSpoke) { *out = *in diff --git a/fleetconfig-controller/charts/fleetconfig-controller/README.md b/fleetconfig-controller/charts/fleetconfig-controller/README.md index 16adb81c..bc220788 100644 --- a/fleetconfig-controller/charts/fleetconfig-controller/README.md +++ b/fleetconfig-controller/charts/fleetconfig-controller/README.md @@ -81,6 +81,7 @@ Resource specifications for all klusterlet-managed containers. | `fleetConfig.registrationAuth.hubClusterARN` | The ARN of the hub cluster. This is only required if configuring an EKS FleetConfig. Example: "arn:aws:eks:us-west-2::cluster/". | `""` | | `fleetConfig.registrationAuth.autoApprovedARNPatterns` | Optional list of spoke cluster name ARN patterns that the hub will auto-approve. | `[]` | | `fleetConfig.hub.addOnConfigs` | Global add-on configuration for the hub cluster. | `[]` | +| `fleetConfig.hub.hubAddOns` | Built-in add-on configuration for the hub cluster. | `[]` | | `fleetConfig.hub.clusterManager.enabled` | Whether to enable the cluster manager. Set to false if using Singleton Control Plane. | `true` | | `fleetConfig.hub.clusterManager.featureGates.DefaultClusterSet` | DefaultClusterSet feature gate. | `true` | | `fleetConfig.hub.clusterManager.featureGates.ManifestWorkReplicaSet` | ManifestWorkReplicaSet feature gate. | `true` | diff --git a/fleetconfig-controller/charts/fleetconfig-controller/crds/fleetconfig.open-cluster-management.io-crds.yaml b/fleetconfig-controller/charts/fleetconfig-controller/crds/fleetconfig.open-cluster-management.io-crds.yaml index 9be2604d..017460d7 100644 --- a/fleetconfig-controller/charts/fleetconfig-controller/crds/fleetconfig.open-cluster-management.io-crds.yaml +++ b/fleetconfig-controller/charts/fleetconfig-controller/crds/fleetconfig.open-cluster-management.io-crds.yaml @@ -2668,6 +2668,27 @@ spec: - wantStatus type: object type: array + installedHubAddOns: + items: + description: InstalledHubAddOn tracks metadata for each hubAddon + that is successfully installed on the hub. + properties: + bundleVersion: + description: BundleVersion is the bundle version used when installing + the addon. + type: string + name: + description: Name is the name of the addon. + type: string + namespace: + description: Namespace is the namespace that the addon was installed + into. + type: string + required: + - bundleVersion + - name + type: object + type: array joinedSpokes: items: description: JoinedSpoke represents a spoke that has been joined diff --git a/fleetconfig-controller/charts/fleetconfig-controller/templates/fleetconfig.yaml b/fleetconfig-controller/charts/fleetconfig-controller/templates/fleetconfig.yaml index 7581b08c..35a69d5c 100644 --- a/fleetconfig-controller/charts/fleetconfig-controller/templates/fleetconfig.yaml +++ b/fleetconfig-controller/charts/fleetconfig-controller/templates/fleetconfig.yaml @@ -111,4 +111,5 @@ spec: overwrite: {{ .overwrite }} {{- end }} {{- end }} + hubAddOns: {{- toYaml .Values.fleetConfig.hub.hubAddOns | nindent 4 }} {{- end }} diff --git a/fleetconfig-controller/charts/fleetconfig-controller/templates/rbac/manager-rbac.yaml b/fleetconfig-controller/charts/fleetconfig-controller/templates/rbac/manager-rbac.yaml index be04dbab..ce0f3ccc 100644 --- a/fleetconfig-controller/charts/fleetconfig-controller/templates/rbac/manager-rbac.yaml +++ b/fleetconfig-controller/charts/fleetconfig-controller/templates/rbac/manager-rbac.yaml @@ -137,7 +137,7 @@ rules: - "addonplacementscores" - "managedclustersetbindings" - "placements" - verbs: ["get", "list", "watch"] + verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] - apiGroups: ["cluster.open-cluster-management.io"] resources: - "managedclusters/status" diff --git a/fleetconfig-controller/charts/fleetconfig-controller/values.yaml b/fleetconfig-controller/charts/fleetconfig-controller/values.yaml index 452ef820..305ca087 100644 --- a/fleetconfig-controller/charts/fleetconfig-controller/values.yaml +++ b/fleetconfig-controller/charts/fleetconfig-controller/values.yaml @@ -68,6 +68,11 @@ fleetConfig: # clusterRoleBinding: "" # ClusterRoleBinding to apply to the add-on. # hubRegistration: false # Enable the agent to register to the hub cluster. # overwrite: false # Whether to overwrite the add-on if it already exists. + ## @param fleetConfig.hub.hubAddOns Built-in add-on configuration for the hub cluster. + hubAddOns: [] + # - name: "" # Name of the add-on. Must be one of "argocd", "governance-policy-framework" + # installNamespace: "" # Namespace to install the add-on. Not applicable for "argocd" add-on. For "governance-policy-framework" add-on, if not set, defaults to "open-cluster-management" namespace. + # createNamespace: false # Whether to create the namespace for the add-on. ## Configuration for the Cluster Manager on the Hub cluster. clusterManager: ## @descriptionStart diff --git a/fleetconfig-controller/config/crd/bases/fleetconfig.open-cluster-management.io_fleetconfigs.yaml b/fleetconfig-controller/config/crd/bases/fleetconfig.open-cluster-management.io_fleetconfigs.yaml index f0465c3d..1f80450b 100644 --- a/fleetconfig-controller/config/crd/bases/fleetconfig.open-cluster-management.io_fleetconfigs.yaml +++ b/fleetconfig-controller/config/crd/bases/fleetconfig.open-cluster-management.io_fleetconfigs.yaml @@ -2658,6 +2658,27 @@ spec: - wantStatus type: object type: array + installedHubAddOns: + items: + description: InstalledHubAddOn tracks metadata for each hubAddon + that is successfully installed on the hub. + properties: + bundleVersion: + description: BundleVersion is the bundle version used when installing + the addon. + type: string + name: + description: Name is the name of the addon. + type: string + namespace: + description: Namespace is the namespace that the addon was installed + into. + type: string + required: + - bundleVersion + - name + type: object + type: array joinedSpokes: items: description: JoinedSpoke represents a spoke that has been joined diff --git a/fleetconfig-controller/internal/controller/addon.go b/fleetconfig-controller/internal/controller/addon.go index 83ff68a9..f4bb46f8 100644 --- a/fleetconfig-controller/internal/controller/addon.go +++ b/fleetconfig-controller/internal/controller/addon.go @@ -26,10 +26,21 @@ import ( ) const ( + // commands addon = "addon" create = "create" enable = "enable" disable = "disable" + + install = "install" + uninstall = "uninstall" + hubAddon = "hub-addon" + + // accepetd hub addon names + hubAddOnArgoCD = "argocd" + hubAddOnGPF = "governance-policy-framework" + + argocdNamespace = "argocd" ) func handleAddonConfig(ctx context.Context, kClient client.Client, addonC *addonapi.Clientset, fc *v1alpha1.FleetConfig) error { @@ -488,6 +499,182 @@ func handleAddonDisable(ctx context.Context, spokeName string, addons []string) return nil } +// isHubAddOnMatching checks if an installed addon matches a desired addon spec +func isHubAddOnMatching(installed v1alpha1.InstalledHubAddOn, desired v1alpha1.HubAddOn, bundleVersion string) bool { + return installed.Name == desired.Name && + installed.Namespace == desired.InstallNamespace && + installed.BundleVerion == bundleVersion +} + +func handleHubAddons(ctx context.Context, kClient client.Client, addonC *addonapi.Clientset, fc *v1alpha1.FleetConfig) error { + logger := log.FromContext(ctx) + logger.V(0).Info("handleHubAddons", "fleetconfig", fc.Name) + + installedAddOns := fc.Status.DeepCopy().InstalledHubAddOns + desiredAddOns := fc.Spec.HubAddOns + bundleVersion := fc.Spec.Hub.ClusterManager.Source.BundleVersion + + // nothing to do + if len(desiredAddOns) == 0 && len(installedAddOns) == 0 { + logger.V(5).Info("no hub addons to reconcile") + return nil + } + + // Find addons that need to be uninstalled (present in installed, missing from desired or version mismatch) + addonsToUninstall := make([]v1alpha1.InstalledHubAddOn, 0) + for _, installed := range installedAddOns { + found := slices.ContainsFunc(desiredAddOns, func(desired v1alpha1.HubAddOn) bool { + return isHubAddOnMatching(installed, desired, bundleVersion) + }) + if !found { + addonsToUninstall = append(addonsToUninstall, installed) + } + } + + // Find addons that need to be installed (present in desired, missing from installed or version upgrade) + addonsToInstall := make([]v1alpha1.HubAddOn, 0) + for _, desired := range desiredAddOns { + found := slices.ContainsFunc(installedAddOns, func(installed v1alpha1.InstalledHubAddOn) bool { + return isHubAddOnMatching(installed, desired, bundleVersion) + }) + if !found { + addonsToInstall = append(addonsToInstall, desired) + } + } + + // do uninstalls first, then installs + err := handleHubAddonUninstall(ctx, addonsToUninstall) + if err != nil { + return err + } + + err = handleHubAddonInstall(ctx, kClient, addonC, addonsToInstall, bundleVersion) + if err != nil { + return err + } + + // build the new installed addons list + newInstalledAddOns := make([]v1alpha1.InstalledHubAddOn, 0, len(desiredAddOns)) + for _, d := range desiredAddOns { + newInstalledAddOns = append(newInstalledAddOns, v1alpha1.InstalledHubAddOn{ + Name: d.Name, + Namespace: d.InstallNamespace, + BundleVerion: bundleVersion, + }) + } + fc.Status.InstalledHubAddOns = newInstalledAddOns + return nil +} + +func handleHubAddonUninstall(ctx context.Context, addons []v1alpha1.InstalledHubAddOn) error { + if len(addons) == 0 { + return nil + } + + logger := log.FromContext(ctx) + logger.V(0).Info("uninstalling hub addons", "count", len(addons)) + + var errs []error + for _, addon := range addons { + args := []string{ + uninstall, + hubAddon, + fmt.Sprintf("--names=%s", addon.Name), + } + if addon.Namespace != "" { + args = append(args, fmt.Sprintf("--namespace=%s", addon.Namespace)) + } + + logger.V(7).Info("running", "command", clusteradm, "args", args) + cmd := exec.Command(clusteradm, args...) + stdout, stderr, err := exec_utils.CmdWithLogs(ctx, cmd, "waiting for 'clusteradm uninstall hub-addon' to complete...") + if err != nil { + out := append(stdout, stderr...) + outStr := string(out) + errs = append(errs, fmt.Errorf("failed to uninstall hubAddon %s: %v, output: %s", addon.Name, err, outStr)) + continue + } + logger.V(1).Info("uninstalled hub addon", "name", addon.Name, "namespace", addon.Namespace, "output", string(stdout)) + } + + if len(errs) > 0 { + return fmt.Errorf("one or more hub addons were not uninstalled: %v", errs) + } + return nil +} + +func handleHubAddonInstall(ctx context.Context, kClient client.Client, addonC *addonapi.Clientset, addons []v1alpha1.HubAddOn, bundleVersion string) error { + if len(addons) == 0 { + return nil + } + + logger := log.FromContext(ctx) + logger.V(0).Info("installing hub addons", "count", len(addons)) + + var errs []error + for _, addon := range addons { + // Check if already installed (defensive check) + installed, err := isAddonInstalled(ctx, addonC, addon.Name) + if err != nil { + errs = append(errs, fmt.Errorf("failed to check if hubAddon %s is installed: %v", addon.Name, err)) + continue + } + if installed { + logger.V(3).Info("hubAddon already installed, skipping", "name", addon.Name) + continue + } + + // workaround until https://github.com/open-cluster-management-io/clusteradm/pull/510 is merged/released + if addon.Name == hubAddOnArgoCD && addon.CreateNamespace { + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: argocdNamespace, + }, + } + err := kClient.Create(ctx, ns) + if err != nil && !kerrs.IsAlreadyExists(err) { + errs = append(errs, fmt.Errorf("failed to create namespace for hubAddon %s: %v", addon.Name, err)) + continue + } + } + + args := []string{ + install, + hubAddon, + fmt.Sprintf("--names=%s", addon.Name), + fmt.Sprintf("--bundle-version=%s", bundleVersion), + fmt.Sprintf("--create-namespace=%t", addon.CreateNamespace), + } + if addon.InstallNamespace != "" { + args = append(args, fmt.Sprintf("--namespace=%s", addon.InstallNamespace)) + } + cmd := exec.Command(clusteradm, args...) + stdout, stderr, err := exec_utils.CmdWithLogs(ctx, cmd, "waiting for 'clusteradm install hub-addon' to complete...") + if err != nil { + out := append(stdout, stderr...) + outStr := string(out) + errs = append(errs, fmt.Errorf("failed to install hubAddon %s: %v, output: %s", addon.Name, err, outStr)) + continue + } + logger.V(1).Info("installed hubAddon", "name", addon.Name, "output", string(stdout)) + } + if len(errs) > 0 { + return fmt.Errorf("one or more hub addons were not installed: %v", errs) + } + return nil +} + +func isAddonInstalled(ctx context.Context, addonC *addonapi.Clientset, addonName string) (bool, error) { + if _, err := addonC.AddonV1alpha1().ClusterManagementAddOns().Get(ctx, addonName, metav1.GetOptions{}); err != nil { + return false, client.IgnoreNotFound(err) + } + + // we enforce unique names between hubAddOns and addOnConfigs, + // and handle deleting addOnConfigs first + // so if the addon is found here, we can assume it was previously installed by `install hub-addon` + return true, nil +} + func labelPatchData(labels map[string]string) map[string]any { return map[string]any{ "metadata": map[string]any{ diff --git a/fleetconfig-controller/internal/controller/hub.go b/fleetconfig-controller/internal/controller/hub.go index c9c4d0e5..37a87d90 100644 --- a/fleetconfig-controller/internal/controller/hub.go +++ b/fleetconfig-controller/internal/controller/hub.go @@ -94,7 +94,15 @@ func handleHub(ctx context.Context, kClient client.Client, fc *v1alpha1.FleetCon return err } - if len(fc.Spec.AddOnConfigs) > 0 { + err = handleHubAddons(ctx, kClient, addonC, fc) + if err != nil { + fc.SetConditions(true, v1alpha1.NewCondition( + err.Error(), v1alpha1.FleetConfigAddonsConfigured, metav1.ConditionFalse, metav1.ConditionTrue, + )) + return err + } + + if len(fc.Spec.AddOnConfigs)+len(fc.Spec.HubAddOns) > 0 { fc.SetConditions(true, v1alpha1.NewCondition( v1alpha1.FleetConfigAddonsConfigured, v1alpha1.FleetConfigAddonsConfigured, metav1.ConditionTrue, metav1.ConditionTrue, )) From 1226fa66039988d918cbce8b187eca0066d192d3 Mon Sep 17 00:00:00 2001 From: Artur Shad Nik Date: Thu, 21 Aug 2025 16:06:05 -0700 Subject: [PATCH 2/3] chore: typo Signed-off-by: Artur Shad Nik --- fleetconfig-controller/api/v1alpha1/fleetconfig_types.go | 2 +- fleetconfig-controller/internal/controller/addon.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go b/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go index 4e6a010d..04607c4b 100644 --- a/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go +++ b/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go @@ -492,7 +492,7 @@ func (j *JoinedSpoke) conditionName(c int) string { // InstalledHubAddOn tracks metadata for each hubAddon that is successfully installed on the hub. type InstalledHubAddOn struct { // BundleVersion is the bundle version used when installing the addon. - BundleVerion string `json:"bundleVersion"` + BundleVersion string `json:"bundleVersion"` // Name is the name of the addon. Name string `json:"name"` // Namespace is the namespace that the addon was installed into. diff --git a/fleetconfig-controller/internal/controller/addon.go b/fleetconfig-controller/internal/controller/addon.go index f4bb46f8..4efc55d0 100644 --- a/fleetconfig-controller/internal/controller/addon.go +++ b/fleetconfig-controller/internal/controller/addon.go @@ -503,7 +503,7 @@ func handleAddonDisable(ctx context.Context, spokeName string, addons []string) func isHubAddOnMatching(installed v1alpha1.InstalledHubAddOn, desired v1alpha1.HubAddOn, bundleVersion string) bool { return installed.Name == desired.Name && installed.Namespace == desired.InstallNamespace && - installed.BundleVerion == bundleVersion + installed.BundleVersion == bundleVersion } func handleHubAddons(ctx context.Context, kClient client.Client, addonC *addonapi.Clientset, fc *v1alpha1.FleetConfig) error { @@ -557,9 +557,9 @@ func handleHubAddons(ctx context.Context, kClient client.Client, addonC *addonap newInstalledAddOns := make([]v1alpha1.InstalledHubAddOn, 0, len(desiredAddOns)) for _, d := range desiredAddOns { newInstalledAddOns = append(newInstalledAddOns, v1alpha1.InstalledHubAddOn{ - Name: d.Name, - Namespace: d.InstallNamespace, - BundleVerion: bundleVersion, + Name: d.Name, + Namespace: d.InstallNamespace, + BundleVersion: bundleVersion, }) } fc.Status.InstalledHubAddOns = newInstalledAddOns From e53e12a1696ff4294a664aa007c77f9be82e3bcc Mon Sep 17 00:00:00 2001 From: Artur Shad Nik <37195151+arturshadnik@users.noreply.github.com> Date: Thu, 21 Aug 2025 16:19:18 -0700 Subject: [PATCH 3/3] chore: Apply suggestions from code review Co-authored-by: Tyler Gillson Signed-off-by: Artur Shad Nik --- fleetconfig-controller/api/v1alpha1/fleetconfig_types.go | 2 ++ .../charts/fleetconfig-controller/values.yaml | 2 +- fleetconfig-controller/internal/controller/addon.go | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go b/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go index 04607c4b..6621d80f 100644 --- a/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go +++ b/fleetconfig-controller/api/v1alpha1/fleetconfig_types.go @@ -493,8 +493,10 @@ func (j *JoinedSpoke) conditionName(c int) string { type InstalledHubAddOn struct { // BundleVersion is the bundle version used when installing the addon. BundleVersion string `json:"bundleVersion"` + // Name is the name of the addon. Name string `json:"name"` + // Namespace is the namespace that the addon was installed into. Namespace string `json:"namespace,omitempty"` } diff --git a/fleetconfig-controller/charts/fleetconfig-controller/values.yaml b/fleetconfig-controller/charts/fleetconfig-controller/values.yaml index 305ca087..b37c90c4 100644 --- a/fleetconfig-controller/charts/fleetconfig-controller/values.yaml +++ b/fleetconfig-controller/charts/fleetconfig-controller/values.yaml @@ -70,7 +70,7 @@ fleetConfig: # overwrite: false # Whether to overwrite the add-on if it already exists. ## @param fleetConfig.hub.hubAddOns Built-in add-on configuration for the hub cluster. hubAddOns: [] - # - name: "" # Name of the add-on. Must be one of "argocd", "governance-policy-framework" + # - name: "" # Name of the add-on. Must be one of "argocd", "governance-policy-framework" # installNamespace: "" # Namespace to install the add-on. Not applicable for "argocd" add-on. For "governance-policy-framework" add-on, if not set, defaults to "open-cluster-management" namespace. # createNamespace: false # Whether to create the namespace for the add-on. ## Configuration for the Cluster Manager on the Hub cluster. diff --git a/fleetconfig-controller/internal/controller/addon.go b/fleetconfig-controller/internal/controller/addon.go index 4efc55d0..a23d2a1f 100644 --- a/fleetconfig-controller/internal/controller/addon.go +++ b/fleetconfig-controller/internal/controller/addon.go @@ -648,6 +648,7 @@ func handleHubAddonInstall(ctx context.Context, kClient client.Client, addonC *a if addon.InstallNamespace != "" { args = append(args, fmt.Sprintf("--namespace=%s", addon.InstallNamespace)) } + cmd := exec.Command(clusteradm, args...) stdout, stderr, err := exec_utils.CmdWithLogs(ctx, cmd, "waiting for 'clusteradm install hub-addon' to complete...") if err != nil { @@ -658,6 +659,7 @@ func handleHubAddonInstall(ctx context.Context, kClient client.Client, addonC *a } logger.V(1).Info("installed hubAddon", "name", addon.Name, "output", string(stdout)) } + if len(errs) > 0 { return fmt.Errorf("one or more hub addons were not installed: %v", errs) }