diff --git a/bundle/manifests/flows.netobserv.io_flowcollectors.yaml b/bundle/manifests/flows.netobserv.io_flowcollectors.yaml index 2cb8eb44a..fc6817574 100644 --- a/bundle/manifests/flows.netobserv.io_flowcollectors.yaml +++ b/bundle/manifests/flows.netobserv.io_flowcollectors.yaml @@ -8033,8 +8033,8 @@ spec: type: object type: array namespace: - description: Namespace where console plugin and flowlogs-pipeline - have been deployed. + description: 'Namespace where console plugin and flowlogs-pipeline + have been deployed. Deprecated: annotations are used instead' type: string required: - conditions diff --git a/config/crd/bases/flows.netobserv.io_flowcollectors.yaml b/config/crd/bases/flows.netobserv.io_flowcollectors.yaml index 26751411e..d0c4c7db1 100644 --- a/config/crd/bases/flows.netobserv.io_flowcollectors.yaml +++ b/config/crd/bases/flows.netobserv.io_flowcollectors.yaml @@ -8019,8 +8019,8 @@ spec: type: object type: array namespace: - description: Namespace where console plugin and flowlogs-pipeline - have been deployed. + description: 'Namespace where console plugin and flowlogs-pipeline + have been deployed. Deprecated: annotations are used instead' type: string required: - conditions diff --git a/controllers/flp/flp_controller.go b/controllers/flp/flp_controller.go index f5273a06b..22c5568aa 100644 --- a/controllers/flp/flp_controller.go +++ b/controllers/flp/flp_controller.go @@ -64,6 +64,8 @@ type subReconciler interface { getStatus() *status.Instance } +// Reconcile is the controller entry point for reconciling current state with desired state. +// It manages the controller status at a high level. Business logic is delegated into `reconcile`. func (r *Reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) { l := log.Log.WithName("flp") // clear context (too noisy) ctx = log.IntoContext(ctx, l) @@ -114,6 +116,8 @@ func (r *Reconciler) reconcile(ctx context.Context) error { } // Create sub-reconcilers + // TODO: refactor to move these subReconciler allocations in `Start`. It will involve some decoupling work, as currently + // `reconcilers.Common` is dependent on the FlowCollector object, which isn't known at start time. reconcilers := []subReconciler{ newMonolithReconciler(cmn.NewInstance(r.mgr.Config.FlowlogsPipelineImage, r.mgr.Status.ForComponent(status.FLPMonolith))), newTransformerReconciler(cmn.NewInstance(r.mgr.Config.FlowlogsPipelineImage, r.mgr.Status.ForComponent(status.FLPTransformOnly))), diff --git a/controllers/flp/flp_test.go b/controllers/flp/flp_test.go index e7fb07c2e..d7029b209 100644 --- a/controllers/flp/flp_test.go +++ b/controllers/flp/flp_test.go @@ -370,7 +370,7 @@ func TestDeploymentNoChange(t *testing.T) { second := b.deployment(annotate(digest)) report := helper.NewChangeReport("") - assert.False(helper.DeploymentChanged(first, second, constants.FLPName, helper.HPADisabled(&cfg.Processor.KafkaConsumerAutoscaler), *cfg.Processor.KafkaConsumerReplicas, &report)) + assert.False(helper.DeploymentChanged(first, second, constants.FLPName, !helper.HPAEnabled(&cfg.Processor.KafkaConsumerAutoscaler), *cfg.Processor.KafkaConsumerReplicas, &report)) assert.Contains(report.String(), "no change") } @@ -394,7 +394,7 @@ func TestDeploymentChanged(t *testing.T) { report := helper.NewChangeReport("") checkChanged := func(old, new *appsv1.Deployment, spec flowslatest.FlowCollectorSpec) bool { - return helper.DeploymentChanged(old, new, constants.FLPName, helper.HPADisabled(&spec.Processor.KafkaConsumerAutoscaler), *spec.Processor.KafkaConsumerReplicas, &report) + return helper.DeploymentChanged(old, new, constants.FLPName, !helper.HPAEnabled(&spec.Processor.KafkaConsumerAutoscaler), *spec.Processor.KafkaConsumerReplicas, &report) } assert.True(checkChanged(first, second, cfg)) @@ -475,7 +475,7 @@ func TestDeploymentChangedReplicasNoHPA(t *testing.T) { second := b.deployment(annotate(digest)) report := helper.NewChangeReport("") - assert.True(helper.DeploymentChanged(first, second, constants.FLPName, helper.HPADisabled(&cfg2.Processor.KafkaConsumerAutoscaler), *cfg2.Processor.KafkaConsumerReplicas, &report)) + assert.True(helper.DeploymentChanged(first, second, constants.FLPName, !helper.HPAEnabled(&cfg2.Processor.KafkaConsumerAutoscaler), *cfg2.Processor.KafkaConsumerReplicas, &report)) assert.Contains(report.String(), "Replicas changed") } diff --git a/controllers/monitoring/monitoring_controller.go b/controllers/monitoring/monitoring_controller.go index bf135f011..0b5dfd376 100644 --- a/controllers/monitoring/monitoring_controller.go +++ b/controllers/monitoring/monitoring_controller.go @@ -39,6 +39,8 @@ func Start(ctx context.Context, mgr *manager.Manager) error { Complete(&r) } +// Reconcile is the controller entry point for reconciling current state with desired state. +// It manages the controller status at a high level. Business logic is delegated into `reconcile`. func (r *Reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) { l := log.Log.WithName("monitoring") // clear context (too noisy) ctx = log.IntoContext(ctx, l) diff --git a/controllers/reconcilers/reconcilers.go b/controllers/reconcilers/reconcilers.go index 50a642108..89e5a5db8 100644 --- a/controllers/reconcilers/reconcilers.go +++ b/controllers/reconcilers/reconcilers.go @@ -168,7 +168,7 @@ func ReconcileDeployment(ctx context.Context, ci *Instance, old, new *appsv1.Dep return ci.CreateOwned(ctx, new) } ci.Status.CheckDeploymentProgress(old) - if helper.DeploymentChanged(old, new, containerName, helper.HPADisabled(hpa), replicas, report) { + if helper.DeploymentChanged(old, new, containerName, !helper.HPAEnabled(hpa), replicas, report) { return ci.UpdateIfOwned(ctx, old, new) } return nil @@ -176,15 +176,14 @@ func ReconcileDeployment(ctx context.Context, ci *Instance, old, new *appsv1.Dep func ReconcileHPA(ctx context.Context, ci *Instance, old, new *ascv2.HorizontalPodAutoscaler, desired *flowslatest.FlowCollectorHPA, report *helper.ChangeReport) error { // Delete or Create / Update Autoscaler according to HPA option - if helper.HPADisabled(desired) { - ci.Managed.TryDelete(ctx, old) - } else { + if helper.HPAEnabled(desired) { if !ci.Managed.Exists(old) { return ci.CreateOwned(ctx, new) } else if helper.AutoScalerChanged(old, *desired, report) { return ci.UpdateIfOwned(ctx, old, new) } } + ci.Managed.TryDelete(ctx, old) return nil } diff --git a/docs/FlowCollector.md b/docs/FlowCollector.md index 71972e7fd..871623d0d 100644 --- a/docs/FlowCollector.md +++ b/docs/FlowCollector.md @@ -14049,7 +14049,7 @@ ResourceClaim references one entry in PodSpec.ResourceClaims. namespace string - Namespace where console plugin and flowlogs-pipeline have been deployed.
+ Namespace where console plugin and flowlogs-pipeline have been deployed. Deprecated: annotations are used instead
false diff --git a/hack/cloned.flows.netobserv.io_flowcollectors.yaml b/hack/cloned.flows.netobserv.io_flowcollectors.yaml index 182101483..411962246 100644 --- a/hack/cloned.flows.netobserv.io_flowcollectors.yaml +++ b/hack/cloned.flows.netobserv.io_flowcollectors.yaml @@ -5567,7 +5567,7 @@ spec: type: object type: array namespace: - description: Namespace where console plugin and flowlogs-pipeline have been deployed. + description: 'Namespace where console plugin and flowlogs-pipeline have been deployed. Deprecated: annotations are used instead' type: string required: - conditions diff --git a/pkg/helper/flowcollector.go b/pkg/helper/flowcollector.go index 779b04b34..d0fbbe04d 100644 --- a/pkg/helper/flowcollector.go +++ b/pkg/helper/flowcollector.go @@ -37,10 +37,6 @@ func HasKafkaExporter(spec *flowslatest.FlowCollectorSpec) bool { return false } -func HPADisabled(spec *flowslatest.FlowCollectorHPA) bool { - return spec == nil || spec.Status == flowslatest.HPAStatusDisabled -} - func HPAEnabled(spec *flowslatest.FlowCollectorHPA) bool { return spec != nil && spec.Status == flowslatest.HPAStatusEnabled }