Merge pull request #224 from slintes/mhc_support

MHC support

Author: openshift-merge-bot[bot]

.github/workflows/pre-submit.yaml

@@ -141,7 +141,7 @@ jobs:
     - name: Run NHC e2e
       run: |
         echo "running e2e test"
-        OPERATOR_NS=${DEPLOY_NAMESPACE} TEST_OPTS='-ginkgo.label-filter="!OCP-ONLY"' make test-e2e
+        OPERATOR_NS=${DEPLOY_NAMESPACE} LABEL_FILTER="!OCP-ONLY" make test-e2e
         echo "finished e2e test"
 
     - name: Debug

Makefile

@@ -364,7 +364,7 @@ bundle-run: operator-sdk ## Run bundle image
 
 .PHONY: bundle-cleanup
 bundle-cleanup: operator-sdk ## Remove bundle installed via bundle-run
-	$(OPERATOR_SDK) -n openshift-operators cleanup node-healthcheck-operator
+	$(OPERATOR_SDK) -n openshift-operators cleanup node-healthcheck-operator --delete-all
 
 # Build a index image by adding bundle images to an empty catalog using the operator package manager tool, 'opm'.
 # This recipe invokes 'opm' in 'semver' bundle add mode. For more information on add modes, see:
@@ -379,11 +379,8 @@ index-push: ## Push a catalog image.
 	podman push $(INDEX_IMG)
 
 .PHONY: test-e2e
-export OPERATOR_NS ?= openshift-operators
 test-e2e: ## Run end to end tests
-	@test -n "${KUBECONFIG}" -o -r ${HOME}/.kube/config || (echo "Failed to find kubeconfig in ~/.kube/config or no KUBECONFIG set"; exit 1)
-	echo "Running e2e tests"
-	go test ./e2e -coverprofile cover.out -timeout 60m -test.v -ginkgo.vv $(TEST_OPTS)
+	./hack/test-e2e.sh
 
 
 .PHONY: deploy-snr ## Deploy self node remediation to a running cluster
@@ -411,4 +408,7 @@ container-build-metrics: ## Build containers
 
 .PHONY: container-push
 container-push:  ## Push containers (NOTE: catalog can't be build before bundle was pushed)
-	make docker-push bundle-push index-build index-push
+	make docker-push bundle-push #index-build index-push
+
+.PHONY: build-and-run
+build-and-run: container-build container-push bundle-run

api/v1alpha1/nodehealthcheck_webhook.go

@@ -28,6 +28,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
 const (
@@ -56,36 +57,36 @@ func (nhc *NodeHealthCheck) SetupWebhookWithManager(mgr ctrl.Manager) error {
 var _ webhook.Validator = &NodeHealthCheck{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
-func (nhc *NodeHealthCheck) ValidateCreate() error {
+func (nhc *NodeHealthCheck) ValidateCreate() (warnings admission.Warnings, err error) {
 	nodehealthchecklog.Info("validate create", "name", nhc.Name)
-	return nhc.validate()
+	return admission.Warnings{}, nhc.validate()
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
-func (nhc *NodeHealthCheck) ValidateUpdate(old runtime.Object) error {
+func (nhc *NodeHealthCheck) ValidateUpdate(old runtime.Object) (warnings admission.Warnings, err error) {
 	nodehealthchecklog.Info("validate update", "name", nhc.Name)
 
 	// do the normal validation
 	if err := nhc.validate(); err != nil {
-		return err
+		return admission.Warnings{}, err
 	}
 
 	// during ongoing remediations, some updates are forbidden
 	if nhc.isRemediating() {
 		if updated, field := nhc.isRestrictedFieldUpdated(old.(*NodeHealthCheck)); updated {
-			return fmt.Errorf("%s update %s", field, OngoingRemediationError)
+			return admission.Warnings{}, fmt.Errorf("%s update %s", field, OngoingRemediationError)
 		}
 	}
-	return nil
+	return admission.Warnings{}, nil
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
-func (nhc *NodeHealthCheck) ValidateDelete() error {
+func (nhc *NodeHealthCheck) ValidateDelete() (warnings admission.Warnings, err error) {
 	nodehealthchecklog.Info("validate delete", "name", nhc.Name)
 	if nhc.isRemediating() {
-		return fmt.Errorf("deletion %s", OngoingRemediationError)
+		return admission.Warnings{}, fmt.Errorf("deletion %s", OngoingRemediationError)
 	}
-	return nil
+	return admission.Warnings{}, nil
 }
 
 func (nhc *NodeHealthCheck) validate() error {

api/v1alpha1/nodehealthcheck_webhook_test.go

@@ -5,10 +5,13 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/types"
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
 var _ = Describe("NodeHealthCheck Validation", func() {
@@ -192,18 +195,25 @@ var _ = Describe("NodeHealthCheck Validation", func() {
 			}
 		})
 
+		validateError := func(validate func(obj runtime.Object) (admission.Warnings, error), nhc *NodeHealthCheck, substrings ...string) {
+			warnings, err := validate(nhc)
+			ExpectWithOffset(1, warnings).To(BeEmpty())
+			matchers := make([]types.GomegaMatcher, 0)
+			for _, substring := range substrings {
+				matchers = append(matchers, ContainSubstring(substring))
+			}
+			ExpectWithOffset(1, err).To(MatchError(
+				And(matchers...),
+			))
+		}
+
 		Context("updating selector", func() {
 			BeforeEach(func() {
 				nhcNew = nhcOld.DeepCopy()
 				nhcNew.Spec.Selector.MatchExpressions[0].Key = "node-role.kubernetes.io/infra"
 			})
 			It("should be denied", func() {
-				Expect(nhcNew.ValidateUpdate(nhcOld)).To(MatchError(
-					And(
-						ContainSubstring(OngoingRemediationError),
-						ContainSubstring("selector"),
-					),
-				))
+				validateError(nhcNew.ValidateUpdate, nhcOld, OngoingRemediationError, "selector")
 			})
 		})
 
@@ -213,12 +223,7 @@ var _ = Describe("NodeHealthCheck Validation", func() {
 				nhcNew.Spec.RemediationTemplate.Name = "newName"
 			})
 			It("should be denied", func() {
-				Expect(nhcNew.ValidateUpdate(nhcOld)).To(MatchError(
-					And(
-						ContainSubstring(OngoingRemediationError),
-						ContainSubstring("remediation template"),
-					),
-				))
+				validateError(nhcNew.ValidateUpdate, nhcOld, OngoingRemediationError, "remediation template")
 			})
 		})
 
@@ -229,12 +234,7 @@ var _ = Describe("NodeHealthCheck Validation", func() {
 				nhcNew.Spec.EscalatingRemediations[0].Order = 42
 			})
 			It("should be denied", func() {
-				Expect(nhcNew.ValidateUpdate(nhcOld)).To(MatchError(
-					And(
-						ContainSubstring(OngoingRemediationError),
-						ContainSubstring("escalating remediations"),
-					),
-				))
+				validateError(nhcNew.ValidateUpdate, nhcOld, OngoingRemediationError, "escalating remediations")
 			})
 		})
 	})

bundle/manifests/node-healthcheck-operator.clusterserviceversion.yaml

@@ -254,6 +254,14 @@ spec:
           - get
           - list
           - watch
+        - apiGroups:
+          - config.openshift.io
+          resources:
+          - featuregates
+          verbs:
+          - get
+          - list
+          - watch
         - apiGroups:
           - console.openshift.io
           resources:
@@ -300,7 +308,17 @@ spec:
           verbs:
           - get
           - list
+          - patch
+          - update
           - watch
+        - apiGroups:
+          - machine.openshift.io
+          resources:
+          - machinehealthchecks/status
+          verbs:
+          - get
+          - patch
+          - update
         - apiGroups:
           - machine.openshift.io
           resources:

config/rbac/role.yaml

@@ -20,6 +20,14 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - config.openshift.io
+  resources:
+  - featuregates
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - console.openshift.io
   resources:
@@ -66,7 +74,17 @@ rules:
   verbs:
   - get
   - list
+  - patch
+  - update
   - watch
+- apiGroups:
+  - machine.openshift.io
+  resources:
+  - machinehealthchecks/status
+  verbs:
+  - get
+  - patch
+  - update
 - apiGroups:
   - machine.openshift.io
   resources: