From a08df981435b67635a2e61200e5868513fc24efe Mon Sep 17 00:00:00 2001 From: Andrei Pavlov Date: Fri, 6 Sep 2024 20:59:24 +0700 Subject: [PATCH 1/2] Remove dependency on hmc-system namespace Signed-off-by: Andrei Pavlov --- api/v1alpha1/management_types.go | 3 +-- api/v1alpha1/template_types.go | 3 --- cmd/main.go | 26 ++++++++++++------- ...roller.go => managedcluster_controller.go} | 9 ++++--- ...t.go => managedcluster_controller_test.go} | 19 ++++++++------ internal/controller/management_controller.go | 19 +++++++------- internal/controller/release_controller.go | 21 ++++++++------- internal/controller/suite_test.go | 5 +++- internal/telemetry/tracker.go | 4 ++- internal/utils/kube.go | 17 ++++++++++++ internal/webhook/managedcluster_webhook.go | 4 ++- .../webhook/managedcluster_webhook_test.go | 7 ++--- 12 files changed, 86 insertions(+), 51 deletions(-) rename internal/controller/{deployment_controller.go => managedcluster_controller.go} (98%) rename internal/controller/{deployment_controller_test.go => managedcluster_controller_test.go} (90%) diff --git a/api/v1alpha1/management_types.go b/api/v1alpha1/management_types.go index 535d0a96e..a53e47ad8 100644 --- a/api/v1alpha1/management_types.go +++ b/api/v1alpha1/management_types.go @@ -30,8 +30,7 @@ const ( } }` - ManagementName = "hmc" - ManagementNamespace = "hmc-system" + ManagementName = "hmc" ManagementFinalizer = "hmc.mirantis.com/management" ) diff --git a/api/v1alpha1/template_types.go b/api/v1alpha1/template_types.go index 64eb8da16..529d3e939 100644 --- a/api/v1alpha1/template_types.go +++ b/api/v1alpha1/template_types.go @@ -22,9 +22,6 @@ import ( ) const ( - // TemplatesNamespace is the namespace where all Templates are located - TemplatesNamespace = "hmc-system" - // ManagementKind is the string representation of a Management. ManagementKind = "Management" // TemplateKind is the string representation of a Template. diff --git a/cmd/main.go b/cmd/main.go index 23053ac15..b155b2cb1 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -169,6 +169,8 @@ func main() { os.Exit(1) } + currentNamespace := utils.CurrentNamespace() + if err = (&controller.TemplateReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), @@ -177,18 +179,20 @@ func main() { os.Exit(1) } if err = (&controller.ManagedClusterReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Config: mgr.GetConfig(), - DynamicClient: dc, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Config: mgr.GetConfig(), + DynamicClient: dc, + SystemNamespace: currentNamespace, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ManagedCluster") os.Exit(1) } if err = (&controller.ManagementReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Config: mgr.GetConfig(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Config: mgr.GetConfig(), + SystemNamespace: currentNamespace, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Management") os.Exit(1) @@ -203,6 +207,7 @@ func main() { RegistryCredentialsSecret: registryCredentialsSecret, InsecureRegistry: insecureRegistry, HMCTemplatesChartName: hmcTemplatesChartName, + SystemNamespace: currentNamespace, }); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ReleaseController") os.Exit(1) @@ -210,7 +215,8 @@ func main() { if enableTelemetry { if err = mgr.Add(&telemetry.Tracker{ - Client: mgr.GetClient(), + Client: mgr.GetClient(), + SystemNamespace: currentNamespace, }); err != nil { setupLog.Error(err, "unable to create telemetry tracker") os.Exit(1) @@ -229,7 +235,9 @@ func main() { } if enableWebhook { - if err := (&hmcwebhook.ManagedClusterValidator{}).SetupWebhookWithManager(mgr); err != nil { + if err := (&hmcwebhook.ManagedClusterValidator{ + SystemNamespace: currentNamespace, + }).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "ManagedCluster") os.Exit(1) } diff --git a/internal/controller/deployment_controller.go b/internal/controller/managedcluster_controller.go similarity index 98% rename from internal/controller/deployment_controller.go rename to internal/controller/managedcluster_controller.go index 00f20029f..d7e2561e6 100644 --- a/internal/controller/deployment_controller.go +++ b/internal/controller/managedcluster_controller.go @@ -53,9 +53,10 @@ import ( // ManagedClusterReconciler reconciles a ManagedCluster object type ManagedClusterReconciler struct { client.Client - Scheme *runtime.Scheme - Config *rest.Config - DynamicClient *dynamic.DynamicClient + Scheme *runtime.Scheme + Config *rest.Config + DynamicClient *dynamic.DynamicClient + SystemNamespace string } // Reconcile is part of the main kubernetes reconciliation loop which aims to @@ -167,7 +168,7 @@ func (r *ManagedClusterReconciler) Update(ctx context.Context, l logr.Logger, ma }() template := &hmc.Template{} - templateRef := types.NamespacedName{Name: managedCluster.Spec.Template, Namespace: hmc.TemplatesNamespace} + templateRef := types.NamespacedName{Name: managedCluster.Spec.Template, Namespace: r.SystemNamespace} if err := r.Get(ctx, templateRef, template); err != nil { l.Error(err, "Failed to get Template") errMsg := fmt.Sprintf("failed to get provided template: %s", err) diff --git a/internal/controller/deployment_controller_test.go b/internal/controller/managedcluster_controller_test.go similarity index 90% rename from internal/controller/deployment_controller_test.go rename to internal/controller/managedcluster_controller_test.go index 59a4c4ea4..edd2625d4 100644 --- a/internal/controller/deployment_controller_test.go +++ b/internal/controller/managedcluster_controller_test.go @@ -30,6 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" hmc "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/internal/utils" ) var _ = Describe("ManagedCluster Controller", func() { @@ -50,11 +51,11 @@ var _ = Describe("ManagedCluster Controller", func() { BeforeEach(func() { By("creating hmc-system namespace") - err := k8sClient.Get(ctx, types.NamespacedName{Name: hmc.ManagementNamespace}, namespace) + err := k8sClient.Get(ctx, types.NamespacedName{Name: utils.DefaultSystemNamespace}, namespace) if err != nil && errors.IsNotFound(err) { namespace = &v1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: hmc.ManagementNamespace, + Name: utils.DefaultSystemNamespace, }, } Expect(k8sClient.Create(ctx, namespace)).To(Succeed()) @@ -66,7 +67,7 @@ var _ = Describe("ManagedCluster Controller", func() { template = &hmc.Template{ ObjectMeta: metav1.ObjectMeta{ Name: templateName, - Namespace: hmc.TemplatesNamespace, + Namespace: utils.DefaultSystemNamespace, }, Spec: hmc.TemplateSpec{ Helm: hmc.HelmSpec{ @@ -123,8 +124,9 @@ var _ = Describe("ManagedCluster Controller", func() { By("Cleanup") controllerReconciler := &ManagedClusterReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), + Client: k8sClient, + Scheme: k8sClient.Scheme(), + SystemNamespace: utils.DefaultSystemNamespace, } Expect(k8sClient.Delete(ctx, managedCluster)).To(Succeed()) @@ -141,9 +143,10 @@ var _ = Describe("ManagedCluster Controller", func() { It("should successfully reconcile the resource", func() { By("Reconciling the created resource") controllerReconciler := &ManagedClusterReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), - Config: &rest.Config{}, + Client: k8sClient, + Scheme: k8sClient.Scheme(), + Config: &rest.Config{}, + SystemNamespace: utils.DefaultSystemNamespace, } _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ diff --git a/internal/controller/management_controller.go b/internal/controller/management_controller.go index 8f69c9f0d..a86be05c6 100644 --- a/internal/controller/management_controller.go +++ b/internal/controller/management_controller.go @@ -43,8 +43,9 @@ import ( // ManagementReconciler reconciles a Management object type ManagementReconciler struct { client.Client - Scheme *runtime.Scheme - Config *rest.Config + Scheme *runtime.Scheme + Config *rest.Config + SystemNamespace string } func (r *ManagementReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -101,26 +102,26 @@ func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Manag for _, component := range components { template := &hmc.Template{} err := r.Get(ctx, types.NamespacedName{ - Namespace: hmc.TemplatesNamespace, + Namespace: r.SystemNamespace, Name: component.Template, }, template) if err != nil { - errMsg := fmt.Sprintf("Failed to get Template %s/%s: %s", hmc.TemplatesNamespace, component.Template, err) + errMsg := fmt.Sprintf("Failed to get Template %s/%s: %s", r.SystemNamespace, component.Template, err) updateComponentsStatus(detectedComponents, &detectedProviders, component.Template, template.Status, errMsg) errs = errors.Join(errs, errors.New(errMsg)) continue } if !template.Status.Valid { - errMsg := fmt.Sprintf("Template %s/%s is not marked as valid", hmc.TemplatesNamespace, component.Template) + errMsg := fmt.Sprintf("Template %s/%s is not marked as valid", r.SystemNamespace, component.Template) updateComponentsStatus(detectedComponents, &detectedProviders, component.Template, template.Status, errMsg) errs = errors.Join(errs, errors.New(errMsg)) continue } - _, _, err = helm.ReconcileHelmRelease(ctx, r.Client, component.HelmReleaseName(), hmc.ManagementNamespace, component.Config, + _, _, err = helm.ReconcileHelmRelease(ctx, r.Client, component.HelmReleaseName(), r.SystemNamespace, component.Config, nil, template.Status.ChartRef, defaultReconcileInterval, component.dependsOn) if err != nil { - errMsg := fmt.Sprintf("error reconciling HelmRelease %s/%s: %s", hmc.ManagementNamespace, component.Template, err) + errMsg := fmt.Sprintf("error reconciling HelmRelease %s/%s: %s", r.SystemNamespace, component.Template, err) updateComponentsStatus(detectedComponents, &detectedProviders, component.Template, template.Status, errMsg) errs = errors.Join(errs, errors.New(errMsg)) continue @@ -169,7 +170,7 @@ func (r *ManagementReconciler) removeHelmReleases(ctx context.Context, hmcReleas l := log.FromContext(ctx) l.Info("Suspending HMC Helm Release reconciles") hmcRelease := &fluxv2.HelmRelease{} - err := r.Client.Get(ctx, types.NamespacedName{Namespace: hmc.ManagementNamespace, Name: hmcReleaseName}, hmcRelease) + err := r.Client.Get(ctx, types.NamespacedName{Namespace: r.SystemNamespace, Name: hmcReleaseName}, hmcRelease) if err != nil && !apierrors.IsNotFound(err) { return err } @@ -251,7 +252,7 @@ func (r *ManagementReconciler) enableAdditionalComponents(ctx context.Context, m capiOperatorValues = config["cluster-api-operator"].(map[string]interface{}) } - err := certmanager.VerifyAPI(ctx, r.Config, r.Scheme, hmc.ManagementNamespace) + err := certmanager.VerifyAPI(ctx, r.Config, r.Scheme, r.SystemNamespace) if err != nil { return fmt.Errorf("failed to check in the cert-manager API is installed: %v", err) } diff --git a/internal/controller/release_controller.go b/internal/controller/release_controller.go index 823826f74..6b63669fb 100644 --- a/internal/controller/release_controller.go +++ b/internal/controller/release_controller.go @@ -66,6 +66,7 @@ type Poller struct { RegistryCredentialsSecret string InsecureRegistry bool HMCTemplatesChartName string + SystemNamespace string } func (p *Poller) Start(ctx context.Context) error { @@ -125,13 +126,13 @@ func (p *Poller) ensureManagement(ctx context.Context) error { }, mgmtObj) if err != nil { if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to get %s/%s Management object", hmc.ManagementNamespace, hmc.ManagementName) + return fmt.Errorf("failed to get %s Management object", hmc.ManagementName) } mgmtObj.Spec.SetProvidersDefaults() getter := helm.NewMemoryRESTClientGetter(p.Config, p.RESTMapper()) actionConfig := new(action.Configuration) - err = actionConfig.Init(getter, hmc.TemplatesNamespace, "secret", l.Info) + err = actionConfig.Init(getter, p.SystemNamespace, "secret", l.Info) if err != nil { return err } @@ -165,7 +166,7 @@ func (p *Poller) ensureManagement(ctx context.Context) error { err = p.Create(ctx, mgmtObj) if err != nil { - return fmt.Errorf("failed to create %s/%s Management object: %s", hmc.ManagementNamespace, hmc.ManagementName, err) + return fmt.Errorf("failed to create %s Management object: %s", hmc.ManagementName, err) } l.Info("Successfully created Management object with default configuration") } @@ -177,7 +178,7 @@ func (p *Poller) reconcileDefaultHelmRepo(ctx context.Context) error { helmRepo := &sourcev1.HelmRepository{ ObjectMeta: metav1.ObjectMeta{ Name: defaultRepoName, - Namespace: hmc.TemplatesNamespace, + Namespace: p.SystemNamespace, }, } operation, err := ctrl.CreateOrUpdate(ctx, p.Client, helmRepo, func() error { @@ -203,7 +204,7 @@ func (p *Poller) reconcileDefaultHelmRepo(ctx context.Context) error { return err } if operation == controllerutil.OperationResultCreated || operation == controllerutil.OperationResultUpdated { - l.Info(fmt.Sprintf("Successfully %s %s/%s HelmRepository", operation, hmc.TemplatesNamespace, defaultRepoName)) + l.Info(fmt.Sprintf("Successfully %s %s/%s HelmRepository", operation, p.SystemNamespace, defaultRepoName)) } return nil } @@ -217,7 +218,7 @@ func (p *Poller) reconcileHMCTemplates(ctx context.Context) error { helmChart := &sourcev1.HelmChart{ ObjectMeta: metav1.ObjectMeta{ Name: p.HMCTemplatesChartName, - Namespace: hmc.TemplatesNamespace, + Namespace: p.SystemNamespace, }, } @@ -241,12 +242,12 @@ func (p *Poller) reconcileHMCTemplates(ctx context.Context) error { return err } if operation == controllerutil.OperationResultCreated || operation == controllerutil.OperationResultUpdated { - l.Info(fmt.Sprintf("Successfully %s %s/%s HelmChart", operation, hmc.TemplatesNamespace, p.HMCTemplatesChartName)) + l.Info(fmt.Sprintf("Successfully %s %s/%s HelmChart", operation, p.SystemNamespace, p.HMCTemplatesChartName)) } err, _ = helm.ArtifactReady(helmChart) if err != nil { - return fmt.Errorf("HelmChart %s/%s Artifact is not ready: %w", hmc.TemplatesNamespace, p.HMCTemplatesChartName, err) + return fmt.Errorf("HelmChart %s/%s Artifact is not ready: %w", p.SystemNamespace, p.HMCTemplatesChartName, err) } chartRef := &hcv2.CrossNamespaceSourceReference{ @@ -254,12 +255,12 @@ func (p *Poller) reconcileHMCTemplates(ctx context.Context) error { Name: helmChart.Name, Namespace: helmChart.Namespace, } - _, operation, err = helm.ReconcileHelmRelease(ctx, p.Client, hmcTemplatesReleaseName, hmc.TemplatesNamespace, nil, nil, chartRef, defaultReconcileInterval, nil) + _, operation, err = helm.ReconcileHelmRelease(ctx, p.Client, hmcTemplatesReleaseName, p.SystemNamespace, nil, nil, chartRef, defaultReconcileInterval, nil) if err != nil { return err } if operation == controllerutil.OperationResultCreated || operation == controllerutil.OperationResultUpdated { - l.Info(fmt.Sprintf("Successfully %s %s/%s HelmRelease", operation, hmc.TemplatesNamespace, hmcTemplatesReleaseName)) + l.Info(fmt.Sprintf("Successfully %s %s/%s HelmRelease", operation, p.SystemNamespace, hmcTemplatesReleaseName)) } return nil } diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 1866af196..691cf93de 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -44,6 +44,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" hmcmirantiscomv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/internal/utils" hmcwebhook "github.com/Mirantis/hmc/internal/webhook" //+kubebuilder:scaffold:imports ) @@ -130,7 +131,9 @@ var _ = BeforeSuite(func() { }) Expect(err).NotTo(HaveOccurred()) - err = (&hmcwebhook.ManagedClusterValidator{}).SetupWebhookWithManager(mgr) + err = (&hmcwebhook.ManagedClusterValidator{ + SystemNamespace: utils.DefaultSystemNamespace, + }).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) err = (&hmcwebhook.ManagementValidator{}).SetupWebhookWithManager(mgr) diff --git a/internal/telemetry/tracker.go b/internal/telemetry/tracker.go index 8eebaaca4..2935c2fb3 100644 --- a/internal/telemetry/tracker.go +++ b/internal/telemetry/tracker.go @@ -30,6 +30,8 @@ import ( type Tracker struct { crclient.Client + + SystemNamespace string } const interval = 10 * time.Minute @@ -69,7 +71,7 @@ func (t *Tracker) trackManagedClusterHeartbeat(ctx context.Context) error { templates := make(map[string]v1alpha1.Template) templatesList := &v1alpha1.TemplateList{} - err = t.List(ctx, templatesList, crclient.InNamespace(v1alpha1.TemplatesNamespace)) + err = t.List(ctx, templatesList, crclient.InNamespace(t.SystemNamespace)) if err != nil { return err } diff --git a/internal/utils/kube.go b/internal/utils/kube.go index 9d98d2dad..7edb047c3 100644 --- a/internal/utils/kube.go +++ b/internal/utils/kube.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "os" apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -25,6 +26,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +const ( + DefaultSystemNamespace = "hmc-system" +) + func EnsureDeleteAllOf(ctx context.Context, cl client.Client, gvk schema.GroupVersionKind, opts *client.ListOptions) error { itemsList := &v1.PartialObjectMetadataList{} itemsList.SetGroupVersionKind(gvk) @@ -43,3 +48,15 @@ func EnsureDeleteAllOf(ctx context.Context, cl client.Client, gvk schema.GroupVe } return errs } + +func CurrentNamespace() string { + ns, found := os.LookupEnv("POD_NAMESPACE") + if found { + return ns + } + nsb, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") + if err == nil && len(nsb) > 0 { + return string(nsb) + } + return DefaultSystemNamespace +} diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go index d61d49c75..bb2b8a26b 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -35,6 +35,8 @@ import ( type ManagedClusterValidator struct { client.Client + + SystemNamespace string } var InvalidManagedClusterErr = errors.New("the ManagedCluster is invalid") @@ -121,7 +123,7 @@ func (v *ManagedClusterValidator) Default(ctx context.Context, obj runtime.Objec func (v *ManagedClusterValidator) getManagedClusterTemplate(ctx context.Context, templateName string) (*v1alpha1.Template, error) { template := &v1alpha1.Template{} - templateRef := types.NamespacedName{Name: templateName, Namespace: v1alpha1.TemplatesNamespace} + templateRef := types.NamespacedName{Name: templateName, Namespace: v.SystemNamespace} if err := v.Get(ctx, templateRef, template); err != nil { return nil, err } diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index 8bcf9e5fb..c17dadaf9 100644 --- a/internal/webhook/managedcluster_webhook_test.go +++ b/internal/webhook/managedcluster_webhook_test.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/internal/utils" "github.com/Mirantis/hmc/test/objects/managedcluster" "github.com/Mirantis/hmc/test/objects/management" "github.com/Mirantis/hmc/test/objects/template" @@ -141,7 +142,7 @@ func TestManagedClusterValidateCreate(t *testing.T) { for _, tt := range createAndUpdateTests { t.Run(tt.name, func(t *testing.T) { c := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(tt.existingObjects...).Build() - validator := &ManagedClusterValidator{Client: c} + validator := &ManagedClusterValidator{Client: c, SystemNamespace: utils.DefaultSystemNamespace} warn, err := validator.ValidateCreate(ctx, tt.managedCluster) if tt.err != "" { g.Expect(err).To(HaveOccurred()) @@ -167,7 +168,7 @@ func TestManagedClusterValidateUpdate(t *testing.T) { for _, tt := range createAndUpdateTests { t.Run(tt.name, func(t *testing.T) { c := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(tt.existingObjects...).Build() - validator := &ManagedClusterValidator{Client: c} + validator := &ManagedClusterValidator{Client: c, SystemNamespace: utils.DefaultSystemNamespace} warn, err := validator.ValidateUpdate(ctx, managedcluster.NewManagedCluster(), tt.managedCluster) if tt.err != "" { g.Expect(err).To(HaveOccurred()) @@ -258,7 +259,7 @@ func TestManagedClusterDefault(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(tt.existingObjects...).Build() - validator := &ManagedClusterValidator{Client: c} + validator := &ManagedClusterValidator{Client: c, SystemNamespace: utils.DefaultSystemNamespace} err := validator.Default(ctx, tt.input) if tt.err != "" { g.Expect(err).To(HaveOccurred()) From 85ace82f465430990edf6d0a1dac4483768f9a90 Mon Sep 17 00:00:00 2001 From: Andrei Pavlov Date: Mon, 9 Sep 2024 16:36:53 +0700 Subject: [PATCH 2/2] Don't override leaderElection ns for cert-manager It will be kube-system by default Signed-off-by: Andrei Pavlov --- templates/hmc/values.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/templates/hmc/values.yaml b/templates/hmc/values.yaml index 1c01884fe..064f4c3f1 100644 --- a/templates/hmc/values.yaml +++ b/templates/hmc/values.yaml @@ -54,9 +54,6 @@ cert-manager: crds: enabled: true keep: false - global: - leaderElection: - namespace: hmc-system flux2: enabled: true