From 4e2f6cd985b4be399241c5576171c588f7281124 Mon Sep 17 00:00:00 2001 From: Courtney Pacheco Date: Mon, 30 Aug 2021 09:14:27 -0400 Subject: [PATCH] Fix prow test failures Remove deadcode, simplify comparisions to bool constants in nodefeaturediscovery_controller.go, and add Security Context Constraints checking. Also remove unused 'updateAvailableCondition' function and check the value of 'err' in the 'applyComponents()' func that was added. --- .../nodefeaturediscovery_controller.go | 38 ++++++---- controllers/nodefeaturediscovery_status.go | 76 +++++++++---------- 2 files changed, 63 insertions(+), 51 deletions(-) diff --git a/controllers/nodefeaturediscovery_controller.go b/controllers/nodefeaturediscovery_controller.go index 4b6e0b85..98373741 100644 --- a/controllers/nodefeaturediscovery_controller.go +++ b/controllers/nodefeaturediscovery_controller.go @@ -141,10 +141,22 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl nfd.init(r, instance) result, err := applyComponents() + if err != nil { + return ctrl.Result{Requeue: true}, err + } + + // Check the status of the NFD Operator Worker SecurityContextConstraints + rstatus, err := r.getSecurityContextConstraintsConditions(ctx) + if rstatus.isDegraded { + return r.updateDegradedCondition(instance, err.Error(), err) + + } else if err != nil { + return r.updateDegradedCondition(instance, conditionFailedGettingNFDScc, err) + } // Check the status of the NFD Operator Worker ServiceAccount - rstatus, err := r.getWorkerServiceAccountConditions(ctx) - if rstatus.isDegraded == true { + rstatus, err = r.getWorkerServiceAccountConditions(ctx) + if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { @@ -153,7 +165,7 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl // Check the status of the NFD Operator Master ServiceAccount rstatus, err = r.getMasterServiceAccountConditions(ctx) - if rstatus.isDegraded == true { + if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { @@ -162,7 +174,7 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl // Check the status of the NFD Operator role rstatus, err = r.getRoleConditions(ctx) - if rstatus.isDegraded == true { + if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { @@ -171,7 +183,7 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl // Check the status of the NFD Operator cluster role rstatus, err = r.getClusterRoleConditions(ctx) - if rstatus.isDegraded == true { + if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { @@ -180,7 +192,7 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl // Check the status of the NFD Operator cluster role binding rstatus, err = r.getClusterRoleBindingConditions(ctx) - if rstatus.isDegraded == true { + if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { @@ -189,7 +201,7 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl // Check the status of the NFD Operator role binding rstatus, err = r.getRoleBindingConditions(ctx) - if rstatus.isDegraded == true { + if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { @@ -198,7 +210,7 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl // Check the status of the NFD Operator Service rstatus, err = r.getServiceConditions(ctx) - if rstatus.isDegraded == true { + if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { @@ -207,7 +219,7 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl // Check the status of the NFD Operator worker ConfigMap rstatus, err = r.getWorkerConfigConditions(nfd) - if rstatus.isDegraded == true { + if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { @@ -216,9 +228,9 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl // Check the status of the NFD Operator Worker DaemonSet rstatus, err = r.getWorkerDaemonSetConditions(ctx) - if rstatus.isProgressing == true { + if rstatus.isProgressing { return r.updateProgressingCondition(instance, err.Error(), err) - } else if rstatus.isDegraded == true { + } else if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { @@ -227,10 +239,10 @@ func (r *NodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, req ctrl // Check the status of the NFD Operator Master DaemonSet rstatus, err = r.getMasterDaemonSetConditions(ctx) - if rstatus.isProgressing == true { + if rstatus.isProgressing { return r.updateProgressingCondition(instance, err.Error(), err) - } else if rstatus.isDegraded == true { + } else if rstatus.isDegraded { return r.updateDegradedCondition(instance, err.Error(), err) } else if err != nil { diff --git a/controllers/nodefeaturediscovery_status.go b/controllers/nodefeaturediscovery_status.go index 60efd3ca..42b100b2 100644 --- a/controllers/nodefeaturediscovery_status.go +++ b/controllers/nodefeaturediscovery_status.go @@ -26,46 +26,33 @@ const ( ) const ( - // Condition failed/degraded messages - conditionReasonValidationFailed = "ValidationFailed" - conditionReasonComponentsCreationFailed = "ComponentCreationFailed" - conditionReasonNFDDegraded = "NFDDegraded" - conditionFailedGettingNFDStatus = "GettingNFDStatusFailed" - // Resource is missing - conditionFailedGettingNFDCustomConfig = "FailedGettingNFDCustomConfig" - conditionFailedGettingNFDOperand = "FailedGettingNFDOperand" - conditionFailedGettingNFDInstance = "FailedGettingNFDInstance" conditionFailedGettingNFDWorkerConfig = "FailedGettingNFDWorkerConfig" conditionFailedGettingNFDWorkerServiceAccount = "FailedGettingNFDWorkerServiceAccount" conditionFailedGettingNFDMasterServiceAccount = "FailedGettingNFDMasterServiceAccount" conditionFailedGettingNFDService = "FailedGettingNFDService" conditionFailedGettingNFDWorkerDaemonSet = "FailedGettingNFDWorkerDaemonSet" conditionFailedGettingNFDMasterDaemonSet = "FailedGettingNFDMasterDaemonSet" - conditionFailedGettingNFDRole = "FailedGettingNFDRole" conditionFailedGettingNFDRoleBinding = "FailedGettingNFDRoleBinding" + conditionFailedGettingNFDScc = "FailedGettingNFDSecurityContextConstraints" // Resource degraded - conditionNFDWorkerConfigDegraded = "NFDWorkerConfigResourceDegraded" - conditionNFDWorkerServiceAccountDegraded = "NFDWorkerServiceAccountDegraded" - conditionNFDMasterServiceAccountDegraded = "NFDMasterServiceAccountDegraded" - conditionNFDServiceDegraded = "NFDServiceDegraded" - conditionNFDWorkerDaemonSetDegraded = "NFDWorkerDaemonSetDegraded" - conditionNFDMasterDaemonSetDegraded = "NFDMasterDaemonSetDegraded" - conditionNFDRoleDegraded = "NFDRoleDegraded" - conditionNFDRoleBindingDegraded = "NFDRoleBindingDegraded" - conditionNFDClusterRoleDegraded = "NFDClusterRoleDegraded" - conditionNFDClusterRoleBindingDegraded = "NFDClusterRoleBindingDegraded" + conditionNFDWorkerConfigDegraded = "NFDWorkerConfigResourceDegraded" + conditionNFDWorkerServiceAccountDegraded = "NFDWorkerServiceAccountDegraded" + conditionNFDMasterServiceAccountDegraded = "NFDMasterServiceAccountDegraded" + conditionNFDServiceDegraded = "NFDServiceDegraded" + conditionNFDWorkerDaemonSetDegraded = "NFDWorkerDaemonSetDegraded" + conditionNFDMasterDaemonSetDegraded = "NFDMasterDaemonSetDegraded" + conditionNFDRoleDegraded = "NFDRoleDegraded" + conditionNFDRoleBindingDegraded = "NFDRoleBindingDegraded" + conditionNFDClusterRoleDegraded = "NFDClusterRoleDegraded" + conditionNFDClusterRoleBindingDegraded = "NFDClusterRoleBindingDegraded" + conditionNFDSecurityContextConstraintsDegraded = "NFDSecurityContextConstraintsDegraded" // Unknown errors. (Catch all) errorNFDWorkerDaemonSetUnknown = "NFDWorkerDaemonSetCorrupted" errorNFDMasterDaemonSetUnknown = "NFDMasterDaemonSetCorrupted" - // Unavailable node errors. (These are triggered when one or - // more nodes are unavailable.) - errorNFDWorkerDaemonSetUnavailableNode = "NFDWorkerDaemonSetUnavailableNode" - errorNFDMasterDaemonSetUnavailableNode = "NFDMasterDaemonSetUnavailableNode" - // Invalid node type. (Denotes that the node should be either // 'worker' or 'master') errorInvalidNodeType = "InvalidNodeTypeSelected" @@ -160,17 +147,6 @@ func (r *NodeFeatureDiscoveryReconciler) updateProgressingCondition(nfd *nfdv1.N return reconcile.Result{Requeue: true}, nil } -// updateAvailableCondition is used to mark a given resource as "progressing" so -// that the reconciler can take steps to rectify the situation. -func (r *NodeFeatureDiscoveryReconciler) updateAvailableCondition(nfd *nfdv1.NodeFeatureDiscovery) (ctrl.Result, error) { - - conditions := r.getAvailableConditions() - if err := r.updateStatus(nfd, conditions); err != nil { - return reconcile.Result{}, err - } - return reconcile.Result{}, errors.New("CouldNotUpdateAvailableConditions") -} - // getAvailableConditions returns a list of conditionsv1.Condition objects and marks // every condition as FALSE except for conditionsv1.ConditionAvailable so that the // reconciler can determine that the resource is available. @@ -311,7 +287,7 @@ func (r *NodeFeatureDiscoveryReconciler) getDaemonSetConditions(ctx context.Cont status := initializeDegradedStatus() // Get the existing DaemonSet from the reconciler - var nodeName string = "" + var nodeName string if node == worker { nodeName = workerName } else if node == master { @@ -539,7 +515,7 @@ func (r *NodeFeatureDiscoveryReconciler) getServiceAccountConditions(ctx context status := initializeDegradedStatus() // Get the existing ServiceAccount from the reconciler - var nodeName string = "" + var nodeName string if node == worker { nodeName = workerName } else if node == master { @@ -565,3 +541,27 @@ func (r *NodeFeatureDiscoveryReconciler) getServiceAccountConditions(ctx context return status, nil } + +// getSecurityContextConstraints gets the current status of a +// SecurityContextConstraints. If an error occurs, this function returns +// the corresponding error message +func (r *NodeFeatureDiscoveryReconciler) getSecurityContextConstraintsConditions(ctx context.Context) (Status, error) { + + // Initialize status to 'Degraded' + status := initializeDegradedStatus() + + // Get the existing SecurityContextConstraints from the reconciler + _, err := r.getSecurityContextConstraints(ctx, nfdNamespace, masterName) + + // If the error is not nil, then the SecurityContextConstraints + // hasn't been (re)created yet + if err != nil { + return status, errors.New(conditionNFDSecurityContextConstraintsDegraded) + } + + // Set the resource to available + status.isAvailable = true + status.isDegraded = false + + return status, nil +}