Skip to content

Commit

Permalink
Merge pull request #135 from mshitrit/same-kind-support
Browse files Browse the repository at this point in the history
Add support to multiple templates with the same kind
  • Loading branch information
openshift-merge-bot[bot] authored Mar 20, 2024
2 parents eae3de2 + 7bbd9c9 commit 85b6f24
Show file tree
Hide file tree
Showing 12 changed files with 160 additions and 40 deletions.
1 change: 1 addition & 0 deletions PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ resources:
path: github.com/medik8s/fence-agents-remediation/api/v1alpha1
version: v1alpha1
webhooks:
defaulting: true
validation: true
webhookVersion: v1
version: "3"
1 change: 1 addition & 0 deletions api/v1alpha1/fenceagentsremediation_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func (r *FenceAgentsRemediation) SetupWebhookWithManager(mgr ctrl.Manager) error

// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
// +kubebuilder:webhook:path=/validate-fence-agents-remediation-medik8s-io-v1alpha1-fenceagentsremediation,mutating=false,failurePolicy=fail,sideEffects=None,groups=fence-agents-remediation.medik8s.io,resources=fenceagentsremediations,verbs=create;update,versions=v1alpha1,name=vfenceagentsremediation.kb.io,admissionReviewVersions=v1

var _ webhook.Validator = &FenceAgentsRemediation{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
Expand Down
18 changes: 18 additions & 0 deletions api/v1alpha1/fenceagentsremediationtemplate_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package v1alpha1

import (
commonAnnotations "github.com/medik8s/common/pkg/annotations"

"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
Expand All @@ -37,8 +39,24 @@ func (r *FenceAgentsRemediationTemplate) SetupWebhookWithManager(mgr ctrl.Manage

// TODO(user): EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!

// +kubebuilder:webhook:path=/mutate-fence-agents-remediation-medik8s-io-v1alpha1-fenceagentsremediationtemplate,mutating=true,failurePolicy=fail,sideEffects=None,groups=fence-agents-remediation.medik8s.io,resources=fenceagentsremediationtemplates,verbs=create;update,versions=v1alpha1,name=mfenceagentsremediationtemplate.kb.io,admissionReviewVersions=v1

var _ webhook.Defaulter = &FenceAgentsRemediationTemplate{}

// Default implements webhook.Defaulter so a webhook will be registered for the type
func (farTemplate *FenceAgentsRemediationTemplate) Default() {
webhookFARTemplateLog.Info("default", "name", farTemplate.Name)
if farTemplate.GetAnnotations() == nil {
farTemplate.Annotations = make(map[string]string)
}
if _, isSameKindAnnotationSet := farTemplate.GetAnnotations()[commonAnnotations.MultipleTemplatesSupportedAnnotation]; !isSameKindAnnotationSet {
farTemplate.Annotations[commonAnnotations.MultipleTemplatesSupportedAnnotation] = "true"
}
}

// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
// +kubebuilder:webhook:path=/validate-fence-agents-remediation-medik8s-io-v1alpha1-fenceagentsremediationtemplate,mutating=false,failurePolicy=fail,sideEffects=None,groups=fence-agents-remediation.medik8s.io,resources=fenceagentsremediationtemplates,verbs=create;update,versions=v1alpha1,name=vfenceagentsremediationtemplate.kb.io,admissionReviewVersions=v1

var _ webhook.Validator = &FenceAgentsRemediationTemplate{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,26 @@ spec:
replaces: fence-agents-remediation.v0.0.1
version: 0.0.1
webhookdefinitions:
- admissionReviewVersions:
- v1
containerPort: 443
deploymentName: fence-agents-remediation-controller-manager
failurePolicy: Fail
generateName: mfenceagentsremediationtemplate.kb.io
rules:
- apiGroups:
- fence-agents-remediation.medik8s.io
apiVersions:
- v1alpha1
operations:
- CREATE
- UPDATE
resources:
- fenceagentsremediationtemplates
sideEffects: None
targetPort: 9443
type: MutatingAdmissionWebhook
webhookPath: /mutate-fence-agents-remediation-medik8s-io-v1alpha1-fenceagentsremediationtemplate
- admissionReviewVersions:
- v1
containerPort: 443
Expand Down
26 changes: 26 additions & 0 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
---
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
name: mutating-webhook-configuration
webhooks:
- admissionReviewVersions:
- v1
clientConfig:
service:
name: webhook-service
namespace: system
path: /mutate-fence-agents-remediation-medik8s-io-v1alpha1-fenceagentsremediationtemplate
failurePolicy: Fail
name: mfenceagentsremediationtemplate.kb.io
rules:
- apiGroups:
- fence-agents-remediation.medik8s.io
apiVersions:
- v1alpha1
operations:
- CREATE
- UPDATE
resources:
- fenceagentsremediationtemplates
sideEffects: None
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
name: validating-webhook-configuration
Expand Down
40 changes: 26 additions & 14 deletions controllers/fenceagentsremediation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct

// Validate FAR CR name to match a nodeName from the cluster
r.Log.Info("Check FAR CR's name")
node, err := utils.GetNodeWithName(r.Client, req.Name)
node, err := utils.GetNodeWithName(r.Client, getNodeName(far))
if err != nil {
r.Log.Error(err, "Unexpected error when validating CR's name with nodes' names", "CR's Name", req.Name)
return emptyResult, err
}
if node == nil {
r.Log.Error(err, "Didn't find a node matching the CR's name", "CR's Name", req.Name)
r.Log.Error(err, "Could not find CR's target node", "CR's Name", req.Name, "Expected node name", getNodeName(far))
utils.UpdateConditions(utils.RemediationFinishedNodeNotFound, far, r.Log)
commonEvents.WarningEvent(r.Recorder, far, utils.EventReasonCrNodeNotFound, utils.EventMessageCrNodeNotFound)
return emptyResult, err
Expand Down Expand Up @@ -168,7 +168,7 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct

// remove node's taints
taint := utils.CreateRemediationTaint()
if err := utils.RemoveTaint(r.Client, far.Name, taint); err != nil {
if err := utils.RemoveTaint(r.Client, node.Name, taint); err != nil {
if apiErrors.IsConflict(err) {
r.Log.Info("Failed to remove taint from node due to node update, retrying... ,", "node name", node.Name, "taint key", taint.Key, "taint effect", taint.Effect)
return ctrl.Result{RequeueAfter: time.Second}, nil
Expand All @@ -179,7 +179,7 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
}
}

r.Log.Info("FAR remediation taint was removed", "Node Name", req.Name)
r.Log.Info("FAR remediation taint was removed", "Node Name", node.Name)
commonEvents.NormalEvent(r.Recorder, node, utils.EventReasonRemoveRemediationTaint, utils.EventMessageRemoveRemediationTaint)
// remove finalizer
controllerutil.RemoveFinalizer(far, v1alpha1.FARFinalizer)
Expand All @@ -191,11 +191,11 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
return emptyResult, nil
}
// Add FAR (medik8s) remediation taint
taintAdded, err := utils.AppendTaint(r.Client, far.Name)
taintAdded, err := utils.AppendTaint(r.Client, node.Name)
if err != nil {
return emptyResult, err
} else if taintAdded {
r.Log.Info("FAR remediation taint was added", "Node Name", req.Name)
r.Log.Info("FAR remediation taint was added", "Node Name", node.Name)
commonEvents.NormalEvent(r.Recorder, node, utils.EventReasonAddRemediationTaint, utils.EventMessageAddRemediationTaint)
}

Expand All @@ -204,19 +204,19 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
// The remeditation has already been processed, thus we can begin with executing the FA for the node

if r.Executor.Exists(far.GetUID()) {
r.Log.Info("A Fence Agent is already running", "Fence Agent", far.Spec.Agent, "Node Name", req.Name, "FAR uid", far.GetUID())
r.Log.Info("A Fence Agent is already running", "Fence Agent", far.Spec.Agent, "Node Name", node.Name, "FAR uid", far.GetUID())
return emptyResult, nil
}

r.Log.Info("Build fence agent command line", "Fence Agent", far.Spec.Agent, "Node Name", req.Name)
r.Log.Info("Build fence agent command line", "Fence Agent", far.Spec.Agent, "Node Name", node.Name)
faParams, err := buildFenceAgentParams(far)
if err != nil {
r.Log.Error(err, "Invalid shared or node parameters from CR", "Name", req.Name)
r.Log.Error(err, "Invalid shared or node parameters from CR", "Node Name", node.Name, "CR Name", req.Name)
return emptyResult, nil
}

cmd := append([]string{far.Spec.Agent}, faParams...)
r.Log.Info("Execute the fence agent", "Fence Agent", far.Spec.Agent, "Node Name", req.Name, "FAR uid", far.GetUID())
r.Log.Info("Execute the fence agent", "Fence Agent", far.Spec.Agent, "Node Name", node.Name, "FAR uid", far.GetUID())
r.Executor.AsyncExecute(ctx, far.GetUID(), cmd, far.Spec.RetryCount, far.Spec.RetryInterval.Duration, far.Spec.Timeout.Duration)
commonEvents.NormalEvent(r.Recorder, far, utils.EventReasonFenceAgentExecuted, utils.EventMessageFenceAgentExecuted)
return emptyResult, nil
Expand All @@ -228,16 +228,16 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
// - try to remove workloads
// - clean up Executor routine

r.Log.Info("Manual workload deletion", "Node Name", req.Name)
r.Log.Info("Manual workload deletion", "Node Name", node.Name)
commonEvents.NormalEvent(r.Recorder, node, utils.EventReasonDeleteResources, utils.EventMessageDeleteResources)
if err := commonResources.DeletePods(ctx, r.Client, req.Name); err != nil {
if err := commonResources.DeletePods(ctx, r.Client, node.Name); err != nil {
r.Log.Error(err, "Manual workload deletion has failed", "CR's Name", req.Name)
return emptyResult, err
}
utils.UpdateConditions(utils.RemediationFinishedSuccessfully, far, r.Log)

r.Executor.Remove(far.GetUID())
r.Log.Info("FenceAgentsRemediation CR has completed to remediate the node", "Node Name", req.Name)
r.Log.Info("FenceAgentsRemediation CR has completed to remediate the node", "Node Name", node.Name)
commonEvents.NormalEvent(r.Recorder, node, utils.EventReasonNodeRemediationCompleted, utils.EventMessageNodeRemediationCompleted)
commonEvents.RemediationFinished(r.Recorder, far)
}
Expand Down Expand Up @@ -287,6 +287,18 @@ func (r *FenceAgentsRemediationReconciler) updateStatus(ctx context.Context, far
return nil
}

// getNodeName checks for the node name in far's commonAnnotations.NodeNameAnnotation if it does not exist it assumes the node name equals to far CR's name and return it.
func getNodeName(far *v1alpha1.FenceAgentsRemediation) string {
ann := far.GetAnnotations()
if ann == nil {
return far.GetName()
}
if nodeName, isNodeNameAnnotationExist := ann[commonAnnotations.NodeNameAnnotation]; isNodeNameAnnotationExist {
return nodeName
}
return far.GetName()
}

// buildFenceAgentParams collects the FAR's parameters for the node based on FAR CR, and if the CR is missing parameters
// or the CR's name don't match nodeParameter name or it has an action which is different than reboot, then return an error
func buildFenceAgentParams(far *v1alpha1.FenceAgentsRemediation) ([]string, error) {
Expand Down Expand Up @@ -314,7 +326,7 @@ func buildFenceAgentParams(far *v1alpha1.FenceAgentsRemediation) ([]string, erro
fenceAgentParams = appendParamToSlice(fenceAgentParams, parameterActionName, parameterActionValue)

// append node parameters
nodeName := v1alpha1.NodeName(far.Name)
nodeName := v1alpha1.NodeName(getNodeName(far))
for paramName, nodeMap := range far.Spec.NodeParameters {
if nodeVal, isFound := nodeMap[nodeName]; isFound {
fenceAgentParams = appendParamToSlice(fenceAgentParams, paramName, nodeVal)
Expand Down
38 changes: 25 additions & 13 deletions controllers/fenceagentsremediation_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,8 @@ var _ = Describe("FAR Controller", func() {
})

When("creating valid FAR CR", func() {
BeforeEach(func() {
node = utils.GetNode("", workerNode)
underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam)
})

It("should have finalizer, taint, while the two VAs and one pod will be deleted", func() {
testSuccessfulRemediation := func() {
Eventually(func(g Gomega) {
g.Expect(storedCommand).To(ConsistOf([]string{
"fence_ipmilan",
Expand All @@ -180,7 +176,7 @@ var _ = Describe("FAR Controller", func() {
"--ipport=6233"}))
}, timeoutPreRemediation, pollInterval).Should(Succeed())

underTestFAR = verifyPreRemediationSucceed(underTestFAR, workerNode, defaultNamespace, &farRemediationTaint)
underTestFAR = verifyPreRemediationSucceed(underTestFAR, defaultNamespace, &farRemediationTaint)

By("Not having any test pod")
verifyPodDeleted(testPodName)
Expand All @@ -194,7 +190,23 @@ var _ = Describe("FAR Controller", func() {
conditionStatusPointer(metav1.ConditionTrue)) // SucceededTypeStatus
verifyEvent(corev1.EventTypeNormal, utils.EventReasonFenceAgentSucceeded, utils.EventMessageFenceAgentSucceeded)
verifyEvent(corev1.EventTypeNormal, utils.EventReasonNodeRemediationCompleted, utils.EventMessageNodeRemediationCompleted)
}
BeforeEach(func() {
node = utils.GetNode("", workerNode)
underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam)
})
When("node name is stored in remediation name", func() {
It("should have finalizer, taint, while the two VAs and one pod will be deleted", testSuccessfulRemediation)
})
//remediation is created from escalation remediation supporting same kind template
When("node name is stored in remediation's annotation", func() {
BeforeEach(func() {
underTestFAR.Name = fmt.Sprintf("%s-%s", workerNode, "pseudo-random-test-sufix")
underTestFAR.Annotations = map[string]string{"remediation.medik8s.io/node-name": workerNode}
})
It("should have finalizer, taint, while the two VAs and one pod will be deleted", testSuccessfulRemediation)
})

})

When("creating invalid FAR CR Name", func() {
Expand Down Expand Up @@ -255,7 +267,7 @@ var _ = Describe("FAR Controller", func() {
})

It("should exit immediately without trying to update the status conditions", func() {
underTestFAR = verifyPreRemediationSucceed(underTestFAR, workerNode, defaultNamespace, &farRemediationTaint)
underTestFAR = verifyPreRemediationSucceed(underTestFAR, defaultNamespace, &farRemediationTaint)

By("Wait some retries")
Eventually(func() int {
Expand Down Expand Up @@ -285,7 +297,7 @@ var _ = Describe("FAR Controller", func() {
})

It("should exit immediately without trying to update the status conditions", func() {
underTestFAR = verifyPreRemediationSucceed(underTestFAR, workerNode, defaultNamespace, &farRemediationTaint)
underTestFAR = verifyPreRemediationSucceed(underTestFAR, defaultNamespace, &farRemediationTaint)

By("Deleting FAR CR")
Expect(k8sClient.Delete(context.Background(), underTestFAR)).To(Succeed())
Expand All @@ -308,7 +320,7 @@ var _ = Describe("FAR Controller", func() {
})

It("should retry the fence agent command as configured and update the status accordingly", func() {
underTestFAR = verifyPreRemediationSucceed(underTestFAR, workerNode, defaultNamespace, &farRemediationTaint)
underTestFAR = verifyPreRemediationSucceed(underTestFAR, defaultNamespace, &farRemediationTaint)

By("Still having one test pod")
verifyPodExists(testPodName)
Expand Down Expand Up @@ -338,7 +350,7 @@ var _ = Describe("FAR Controller", func() {
})

It("should stop Fence Agent execution and update the status accordingly", func() {
underTestFAR = verifyPreRemediationSucceed(underTestFAR, workerNode, defaultNamespace, &farRemediationTaint)
underTestFAR = verifyPreRemediationSucceed(underTestFAR, defaultNamespace, &farRemediationTaint)

By("Still having one test pod")
verifyPodExists(testPodName)
Expand Down Expand Up @@ -492,16 +504,16 @@ func verifyStatusCondition(far *v1alpha1.FenceAgentsRemediation, nodeName, condi
}

// verifyPreRemediationSucceed checks if the remediation CR already has a finazliaer and a remediation taint
func verifyPreRemediationSucceed(underTestFAR *v1alpha1.FenceAgentsRemediation, nodeName, namespace string, taint *corev1.Taint) *v1alpha1.FenceAgentsRemediation {
func verifyPreRemediationSucceed(underTestFAR *v1alpha1.FenceAgentsRemediation, namespace string, taint *corev1.Taint) *v1alpha1.FenceAgentsRemediation {
By("Searching for finalizer ")
Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: nodeName, Namespace: namespace}, underTestFAR)).To(Succeed())
Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: underTestFAR.GetName(), Namespace: namespace}, underTestFAR)).To(Succeed())
Expect(controllerutil.ContainsFinalizer(underTestFAR, v1alpha1.FARFinalizer)).To(BeTrue())
verifyEvent(corev1.EventTypeNormal, utils.EventReasonRemediationStarted, utils.EventMessageRemediationStarted)

By("Searching for remediation taint if we have a finalizer")
Eventually(func(g Gomega) {
node := &corev1.Node{}
g.Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: nodeName}, node)).To(Succeed())
g.Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: getNodeName(underTestFAR)}, node)).To(Succeed())
g.Expect(utils.TaintExists(node.Spec.Taints, taint)).To(BeTrue(), "remediation taint should exist")
}, timeoutPreRemediation, pollInterval).Should(Succeed())
verifyEvent(corev1.EventTypeNormal, utils.EventReasonAddRemediationTaint, utils.EventMessageAddRemediationTaint)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.20

require (
github.com/go-logr/logr v1.4.1
github.com/medik8s/common v1.12.0
github.com/medik8s/common v1.17.0
github.com/onsi/ginkgo/v2 v2.14.0
github.com/onsi/gomega v1.30.0
github.com/openshift/api v0.0.0-20230621174358-ea40115b9fa6
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0
github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg=
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k=
github.com/medik8s/common v1.12.0 h1:UJ5VS4rbo/K0snfuqRiYam1NhXTwo4Q7fTng34YSlmA=
github.com/medik8s/common v1.12.0/go.mod h1:Q6YR2rgyiLl6ob4Mx2uDBmybAB3d94fImwoHPbeiE54=
github.com/medik8s/common v1.17.0 h1:AmJKx0tzqGZF27Ot0A4ak85q0F0zqUkVyCvYmm67rtY=
github.com/medik8s/common v1.17.0/go.mod h1:A9jYldC6PZcAuBowNNm712FqWdASB2ey5Vjp8MYN/PY=
github.com/moby/spdystream v0.2.0 h1:cjW1zVyyoiM0T7b6UoySUFqzXMoqRckQtXwGPiBhOM8=
github.com/moby/spdystream v0.2.0/go.mod h1:f7i0iNDQJ059oMTcWxx8MA/zKFIuD/lY+0GqbN2Wy8c=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 85b6f24

Please sign in to comment.