Respond to comments

Signed-off-by: Artyom Lukianov <[email protected]>

cmd/manager/main.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"flag"
 	"fmt"
-	"os"
 	"runtime"
 
 	"github.com/openshift-kni/performance-addon-operators/pkg/apis"
@@ -65,8 +64,7 @@ func main() {
 
 	namespace, err := k8sutil.GetWatchNamespace()
 	if err != nil {
-		klog.Error(err.Error())
-		os.Exit(1)
+		klog.Exit(err.Error())
 	}
 
 	// we have two namespaces that we need to watch
@@ -80,8 +78,7 @@ func main() {
 	// Get a config to talk to the apiserver
 	cfg, err := config.GetConfig()
 	if err != nil {
-		klog.Error(err.Error())
-		os.Exit(1)
+		klog.Exit(err.Error())
 	}
 
 	// Create a new Cmd to provide shared dependencies and start components
@@ -95,37 +92,31 @@ func main() {
 		MetricsBindAddress:      fmt.Sprintf("%s:%d", metricsHost, metricsPort),
 	})
 	if err != nil {
-		klog.Error(err.Error())
-		os.Exit(1)
+		klog.Exit(err.Error())
 	}
 
 	klog.Info("Registering Components.")
 
 	// Setup Scheme for all resources
 	if err := apis.AddToScheme(mgr.GetScheme()); err != nil {
-		klog.Error(err.Error())
-		os.Exit(1)
+		klog.Exit(err.Error())
 	}
 
 	if err := configv1.AddToScheme(mgr.GetScheme()); err != nil {
-		klog.Error(err.Error())
-		os.Exit(1)
+		klog.Exit(err.Error())
 	}
 
 	if err := mcov1.AddToScheme(mgr.GetScheme()); err != nil {
-		klog.Error(err.Error())
-		os.Exit(1)
+		klog.Exit(err.Error())
 	}
 
 	if err := tunedv1.AddToScheme(mgr.GetScheme()); err != nil {
-		klog.Error(err.Error())
-		os.Exit(1)
+		klog.Exit(err.Error())
 	}
 
 	// Setup all Controllers
 	if err := controller.AddToManager(mgr); err != nil {
-		klog.Error(err.Error())
-		os.Exit(1)
+		klog.Exit(err.Error())
 	}
 
 	if err = serveCRMetrics(cfg); err != nil {
@@ -160,8 +151,7 @@ func main() {
 
 	// Start the Cmd
 	if err := mgr.Start(signals.SetupSignalHandler()); err != nil {
-		klog.Errorf("Manager exited with non-zero code: %v", err)
-		os.Exit(1)
+		klog.Exitf("Manager exited with non-zero code: %v", err)
 	}
 }
 

hack/clean-deploy.sh

@@ -3,18 +3,7 @@
 # expect oc to be in PATH by default
 OC_TOOL="${OC_TOOL:-oc}"
 
-$OC_TOOL delete -f - <<EOF
-apiVersion: operators.coreos.com/v1alpha1
-kind: Subscription
-metadata:
-  name: performance-addon-operators-subscription
-  namespace: openshift-performance-addon
-spec:
-  channel: alpha
-  name: performance-addon-operators
-  source: performance-addon-operators-catalogsource
-  sourceNamespace: openshift-marketplace
-EOF
+$OC_TOOL delete ns openshift-performance-addon
 
 $OC_TOOL delete -f - <<EOF
 ---
@@ -32,5 +21,3 @@ spec:
   publisher: Red Hat
   sourceType: grpc
 EOF
-
-$OC_TOOL -n openshift-performance-addon delete csv --all

pkg/controller/performanceprofile/components/consts.go

@@ -7,8 +7,8 @@ const (
 const (
 	// LabelMachineConfigurationRole defines the label for machine configuration role
 	LabelMachineConfigurationRole = "machineconfiguration.openshift.io/role"
-	// LableMachineConfigPoolRole defines the label for machine config pool role
-	LableMachineConfigPoolRole = "machineconfigpool.openshift.io/role"
+	// LabelMachineConfigPoolRole defines the label for machine config pool role
+	LabelMachineConfigPoolRole = "machineconfigpool.openshift.io/role"
 	// RoleWorker defines the worker role
 	RoleWorker = "worker"
 	// RoleWorkerPerformance defines the worker role for performance sensitive workflows
@@ -29,6 +29,6 @@ const (
 const (
 	// FeatureGateLatencySensetiveName defines the latency sensetive feature gate name
 	// TOOD: uncomment once https://bugzilla.redhat.com/show_bug.cgi?id=1788061 fixed
-	// FeatureGateLatencySensetiveName = "latency-sensetive"
+	// FeatureGateLatencySensetiveName = "latency-sensitive"
 	FeatureGateLatencySensetiveName = "cluster"
 )

pkg/controller/performanceprofile/components/kubeletconfig/kubeletconfig.go

@@ -53,7 +53,7 @@ func NewPerformance(profile *performancev1alpha1.PerformanceProfile) *machinecon
 		Spec: machineconfigv1.KubeletConfigSpec{
 			MachineConfigPoolSelector: &metav1.LabelSelector{
 				MatchLabels: map[string]string{
-					components.LableMachineConfigPoolRole: name,
+					components.LabelMachineConfigPoolRole: name,
 				},
 			},
 			KubeletConfig: &runtime.RawExtension{

pkg/controller/performanceprofile/components/kubeletconfig/kubeletconfig_test.go

@@ -19,7 +19,7 @@ var _ = Describe("Kubelet Config", func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		manifest := string(y)
-		Expect(manifest).To(ContainSubstring(fmt.Sprintf("%s: %s", components.LableMachineConfigPoolRole, components.RoleWorkerPerformance)))
+		Expect(manifest).To(ContainSubstring(fmt.Sprintf("%s: %s", components.LabelMachineConfigPoolRole, components.RoleWorkerPerformance)))
 		Expect(manifest).To(ContainSubstring("reservedSystemCPUs: 0-3"))
 		Expect(manifest).To(ContainSubstring("topologyManagerPolicy: best-effort"))
 		Expect(manifest).To(ContainSubstring("cpuManagerPolicy: static"))

pkg/controller/performanceprofile/components/machineconfigpool/machineconfigpool.go

@@ -19,7 +19,7 @@ func NewPerformance(profile *performancev1alpha1.PerformanceProfile) *machinecon
 		ObjectMeta: metav1.ObjectMeta{
 			Name: name,
 			Labels: map[string]string{
-				components.LableMachineConfigPoolRole: name,
+				components.LabelMachineConfigPoolRole: name,
 			},
 		},
 		Spec: machineconfigv1.MachineConfigPoolSpec{

pkg/controller/performanceprofile/performanceprofile_controller.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"sync"
+	"time"
 
 	performancev1alpha1 "github.com/openshift-kni/performance-addon-operators/pkg/apis/performance/v1alpha1"
 	"github.com/openshift-kni/performance-addon-operators/pkg/controller/performanceprofile/components"
@@ -129,7 +131,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 		return err
 	}
 
-	// we do not want initiate reconcile loop on the pause of machine config pool
+	// we do not want initiate reconcile loop on the configuration or pause fields update
 	mcpPredicates := predicate.Funcs{
 		UpdateFunc: func(e event.UpdateEvent) bool {
 			if e.MetaOld == nil {
@@ -149,13 +151,14 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 				return false
 			}
 
-			mcpNew := e.ObjectOld.(*mcov1.MachineConfigPool)
-			mcpOld := e.ObjectNew.(*mcov1.MachineConfigPool)
+			mcpOld := e.ObjectOld.(*mcov1.MachineConfigPool)
+			mcpNew := e.ObjectNew.(*mcov1.MachineConfigPool)
+			mcpNewCopy := mcpNew.DeepCopy()
 
-			mcpNew.Spec.Paused = mcpOld.Spec.Paused
-			mcpNew.Spec.Configuration = mcpOld.Spec.Configuration
+			mcpNewCopy.Spec.Paused = mcpOld.Spec.Paused
+			mcpNewCopy.Spec.Configuration = mcpOld.Spec.Configuration
 
-			return !reflect.DeepEqual(mcpOld.Spec, mcpNew.Spec) ||
+			return !reflect.DeepEqual(mcpOld.Spec, mcpNewCopy.Spec) ||
 				!apiequality.Semantic.DeepEqual(e.MetaNew.GetLabels(), e.MetaOld.GetLabels())
 		},
 	}
@@ -182,6 +185,7 @@ type ReconcilePerformanceProfile struct {
 	client    client.Client
 	scheme    *runtime.Scheme
 	assetsDir string
+	pauseLock sync.Mutex
 }
 
 // Reconcile reads that state of the cluster for a PerformanceProfile object and makes changes based on the state read
@@ -215,6 +219,10 @@ func (r *ReconcilePerformanceProfile) Reconcile(request reconcile.Request) (reco
 			return reconcile.Result{}, err
 		}
 
+		if r.isComponentsExist(instance) {
+			return reconcile.Result{Requeue: true, RequeueAfter: time.Minute}, nil
+		}
+
 		// remove finalizer
 		if hasFinalizer(instance, finalizer) {
 			removeFinalizer(instance, finalizer)
@@ -229,7 +237,7 @@ func (r *ReconcilePerformanceProfile) Reconcile(request reconcile.Request) (reco
 	// TODO: we need to check if all under performance profiles values != nil
 	// first we need to decide if each of values required and we should move the check into validation webhook
 	// for now let's assume that all parameters needed for assets scrips are required
-	if err := r.verifyPerformanceProfileParameters(instance); err != nil {
+	if err := r.validatePerformanceProfileParameters(instance); err != nil {
 		// we do not want to reconcile again in case of error, because a user will need to update the PerformanceProfile anyway
 		klog.Errorf("failed to reconcile: %v", err)
 		return reconcile.Result{}, nil
@@ -247,6 +255,8 @@ func (r *ReconcilePerformanceProfile) Reconcile(request reconcile.Request) (reco
 	}
 
 	// apply components
+	r.pauseLock.Lock()
+	defer r.pauseLock.Unlock()
 	if err := r.applyComponents(instance); err != nil {
 		klog.Errorf("failed to deploy components: %v", err)
 		return reconcile.Result{}, err
@@ -257,10 +267,10 @@ func (r *ReconcilePerformanceProfile) Reconcile(request reconcile.Request) (reco
 	return reconcile.Result{}, nil
 }
 
-func (r *ReconcilePerformanceProfile) applyComponents(profle *performancev1alpha1.PerformanceProfile) error {
+func (r *ReconcilePerformanceProfile) applyComponents(profile *performancev1alpha1.PerformanceProfile) error {
 	// deploy machine config pool
-	mcp := machineconfigpool.NewPerformance(profle)
-	if err := controllerutil.SetControllerReference(profle, mcp, r.scheme); err != nil {
+	mcp := machineconfigpool.NewPerformance(profile)
+	if err := controllerutil.SetControllerReference(profile, mcp, r.scheme); err != nil {
 		return err
 	}
 	if err := r.createOrUpdateMachineConfigPool(mcp); err != nil {
@@ -273,12 +283,12 @@ func (r *ReconcilePerformanceProfile) applyComponents(profle *performancev1alpha
 	}
 
 	// deploy machine config
-	mc, err := machineconfig.NewPerformance(r.assetsDir, profle)
+	mc, err := machineconfig.NewPerformance(r.assetsDir, profile)
 	if err != nil {
 		return err
 	}
 
-	if err := controllerutil.SetControllerReference(profle, mc, r.scheme); err != nil {
+	if err := controllerutil.SetControllerReference(profile, mc, r.scheme); err != nil {
 		return err
 	}
 
@@ -287,8 +297,8 @@ func (r *ReconcilePerformanceProfile) applyComponents(profle *performancev1alpha
 	}
 
 	// deploy kubelet config
-	kc := kubeletconfig.NewPerformance(profle)
-	if err := controllerutil.SetControllerReference(profle, kc, r.scheme); err != nil {
+	kc := kubeletconfig.NewPerformance(profile)
+	if err := controllerutil.SetControllerReference(profile, kc, r.scheme); err != nil {
 		return err
 	}
 	if err := r.createOrUpdateKubeletConfig(kc); err != nil {
@@ -298,7 +308,7 @@ func (r *ReconcilePerformanceProfile) applyComponents(profle *performancev1alpha
 	// deploy feature gate
 	fg := featuregate.NewLatencySensitive()
 	// TOOD: uncomment once https://bugzilla.redhat.com/show_bug.cgi?id=1788061 fixed
-	// if err := controllerutil.SetControllerReference(profle, fg, r.scheme); err != nil {
+	// if err := controllerutil.SetControllerReference(profile, fg, r.scheme); err != nil {
 	// 	return err
 	// }
 	if err := r.createOrUpdateFeatureGate(fg); err != nil {
@@ -311,7 +321,7 @@ func (r *ReconcilePerformanceProfile) applyComponents(profle *performancev1alpha
 		return err
 	}
 
-	if err := controllerutil.SetControllerReference(profle, networkLatencyTuned, r.scheme); err != nil {
+	if err := controllerutil.SetControllerReference(profile, networkLatencyTuned, r.scheme); err != nil {
 		return err
 	}
 
@@ -320,12 +330,12 @@ func (r *ReconcilePerformanceProfile) applyComponents(profle *performancev1alpha
 	}
 
 	// deploy real time kernel tuned
-	realTimeKernelTuned, err := tuned.NewWorkerRealTimeKernel(r.assetsDir, profle)
+	realTimeKernelTuned, err := tuned.NewWorkerRealTimeKernel(r.assetsDir, profile)
 	if err != nil {
 		return err
 	}
 
-	if err := controllerutil.SetControllerReference(profle, realTimeKernelTuned, r.scheme); err != nil {
+	if err := controllerutil.SetControllerReference(profile, realTimeKernelTuned, r.scheme); err != nil {
 		return err
 	}
 
@@ -346,9 +356,6 @@ func (r *ReconcilePerformanceProfile) pauseMachineConfigPool(mcpName string, pau
 		return nil
 	}
 
-	if err != nil {
-		return err
-	}
 	if err != nil {
 		return err
 	}
@@ -357,7 +364,7 @@ func (r *ReconcilePerformanceProfile) pauseMachineConfigPool(mcpName string, pau
 	return r.client.Update(context.TODO(), updatedMcp)
 }
 
-func (r *ReconcilePerformanceProfile) verifyPerformanceProfileParameters(performanceProfile *performancev1alpha1.PerformanceProfile) error {
+func (r *ReconcilePerformanceProfile) validatePerformanceProfileParameters(performanceProfile *performancev1alpha1.PerformanceProfile) error {
 	if performanceProfile.Spec.CPU == nil {
 		return fmt.Errorf("you should provide CPU section")
 	}
@@ -411,6 +418,31 @@ func (r *ReconcilePerformanceProfile) deleteComponents(profile *performancev1alp
 	return r.deleteMachineConfigPool(name)
 }
 
+func (r *ReconcilePerformanceProfile) isComponentsExist(profile *performancev1alpha1.PerformanceProfile) bool {
+	tunedName := components.GetComponentName(profile.Name, components.ProfileNameWorkerRT)
+	if _, err := r.getTuned(tunedName, components.NamespaceNodeTuningOperator); !errors.IsNotFound(err) {
+		return true
+	}
+
+	if _, err := r.getTuned(components.ProfileNameNetworkLatency, components.NamespaceNodeTuningOperator); !errors.IsNotFound(err) {
+		return true
+	}
+
+	name := components.GetComponentName(profile.Name, components.RoleWorkerPerformance)
+	if _, err := r.getKubeletConfig(name); !errors.IsNotFound(err) {
+		return true
+	}
+
+	if _, err := r.getMachineConfig(name); !errors.IsNotFound(err) {
+		return true
+	}
+
+	if _, err := r.getMachineConfigPool(name); !errors.IsNotFound(err) {
+		return true
+	}
+	return false
+}
+
 func hasFinalizer(profile *performancev1alpha1.PerformanceProfile, finalizer string) bool {
 	for _, f := range profile.Finalizers {
 		if f == finalizer {