Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle unexpected use case where SNR's configuration is deleted #209

Merged
merged 17 commits into from
Jun 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
11 changes: 9 additions & 2 deletions api/v1alpha1/selfnoderemediation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,17 @@ const (
ResourceDeletionRemediationStrategy = RemediationStrategyType("ResourceDeletion")
OutOfServiceTaintRemediationStrategy = RemediationStrategyType("OutOfServiceTaint")
DefaultRemediationStrategy = AutomaticRemediationStrategy
)

type ConditionType string

const (
// ProcessingConditionType is the condition type used to signal NHC the remediation status
ProcessingConditionType = "Processing"
ProcessingConditionType ConditionType = "Processing"
// SucceededConditionType is the condition type used to signal NHC whether the remediation was successful or not
SucceededConditionType = "Succeeded"
SucceededConditionType ConditionType = "Succeeded"
// DisabledConditionType is the condition type used to signal SNR is disabled
DisabledConditionType ConditionType = "Disabled"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
Expand Down
13 changes: 7 additions & 6 deletions api/v1alpha1/selfnoderemediation_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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"
)

var (
Expand All @@ -38,20 +39,20 @@ func (r *SelfNodeRemediation) SetupWebhookWithManager(mgr ctrl.Manager) error {
var _ webhook.Validator = &SelfNodeRemediation{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *SelfNodeRemediation) ValidateCreate() error {
func (r *SelfNodeRemediation) ValidateCreate() (warning admission.Warnings, err error) {
webhookRemediationLog.Info("validate create", "name", r.Name)
return validateStrategy(r.Spec)
return admission.Warnings{}, validateStrategy(r.Spec)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *SelfNodeRemediation) ValidateUpdate(_ runtime.Object) error {
func (r *SelfNodeRemediation) ValidateUpdate(_ runtime.Object) (warning admission.Warnings, err error) {
webhookRemediationLog.Info("validate update", "name", r.Name)
return validateStrategy(r.Spec)
return admission.Warnings{}, validateStrategy(r.Spec)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *SelfNodeRemediation) ValidateDelete() error {
func (r *SelfNodeRemediation) ValidateDelete() (warning admission.Warnings, err error) {
// unused for now, add "delete" when needed to verbs in the kubebuilder annotation above
webhookRemediationLog.Info("validate delete", "name", r.Name)
return nil
return admission.Warnings{}, nil
}
15 changes: 10 additions & 5 deletions api/v1alpha1/selfnoderemediation_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ var _ = Describe("SelfNodeRemediation Validation", func() {

Context("with valid strategy", func() {
It("should be allowed", func() {
Expect(snrValid.ValidateCreate()).To(Succeed())
_, err := snrValid.ValidateCreate()
Expect(err).To(Succeed())
})
})

Expand All @@ -53,17 +54,21 @@ var _ = Describe("SelfNodeRemediation Validation", func() {
utils.IsOutOfServiceTaintSupported = true
})
It("should be allowed", func() {
Expect(outOfServiceStrategy.ValidateCreate()).To(Succeed())
Expect(snrValid.ValidateUpdate(outOfServiceStrategy)).To(Succeed())
_, err := outOfServiceStrategy.ValidateCreate()
Expect(err).To(Succeed())
_, err = snrValid.ValidateUpdate(outOfServiceStrategy)
Expect(err).To(Succeed())
})
})
When("out of service taint is not supported", func() {
BeforeEach(func() {
utils.IsOutOfServiceTaintSupported = false
})
It("should be denied", func() {
Expect(outOfServiceStrategy.ValidateCreate()).To(MatchError(ContainSubstring("OutOfServiceTaint remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy")))
Expect(outOfServiceStrategy.ValidateUpdate(snrValid)).To(MatchError(ContainSubstring("OutOfServiceTaint remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy")))
_, err := outOfServiceStrategy.ValidateCreate()
Expect(err).To(MatchError(ContainSubstring("OutOfServiceTaint remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy")))
_, err = outOfServiceStrategy.ValidateUpdate(snrValid)
Expect(err).To(MatchError(ContainSubstring("OutOfServiceTaint remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy")))
})
})

Expand Down
24 changes: 16 additions & 8 deletions api/v1alpha1/selfnoderemediationconfig_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,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"

"github.com/medik8s/self-node-remediation/pkg/utils"
)
Expand Down Expand Up @@ -65,15 +66,15 @@ func (r *SelfNodeRemediationConfig) SetupWebhookWithManager(mgr ctrl.Manager) er
Complete()
}

//+kubebuilder:webhook:path=/validate-self-node-remediation-medik8s-io-v1alpha1-selfnoderemediationconfig,mutating=false,failurePolicy=fail,sideEffects=None,groups=self-node-remediation.medik8s.io,resources=selfnoderemediationconfigs,verbs=create;update,versions=v1alpha1,name=vselfnoderemediationconfig.kb.io,admissionReviewVersions={v1}
//+kubebuilder:webhook:path=/validate-self-node-remediation-medik8s-io-v1alpha1-selfnoderemediationconfig,mutating=false,failurePolicy=fail,sideEffects=None,groups=self-node-remediation.medik8s.io,resources=selfnoderemediationconfigs,verbs=create;update;delete,versions=v1alpha1,name=vselfnoderemediationconfig.kb.io,admissionReviewVersions={v1}

var _ webhook.Validator = &SelfNodeRemediationConfig{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *SelfNodeRemediationConfig) ValidateCreate() error {
func (r *SelfNodeRemediationConfig) ValidateCreate() (warning admission.Warnings, err error) {
selfNodeRemediationConfigLog.Info("validate create", "name", r.Name)

return errors.NewAggregate([]error{
return admission.Warnings{}, errors.NewAggregate([]error{
r.validateTimes(),
r.validateCustomTolerations(),
r.validateSingleton(),
Expand All @@ -82,20 +83,27 @@ func (r *SelfNodeRemediationConfig) ValidateCreate() error {
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *SelfNodeRemediationConfig) ValidateUpdate(_ runtime.Object) error {
func (r *SelfNodeRemediationConfig) ValidateUpdate(_ runtime.Object) (warning admission.Warnings, err error) {
selfNodeRemediationConfigLog.Info("validate update", "name", r.Name)

return errors.NewAggregate([]error{
return admission.Warnings{}, errors.NewAggregate([]error{
r.validateTimes(),
r.validateCustomTolerations(),
})
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *SelfNodeRemediationConfig) ValidateDelete() error {
func (r *SelfNodeRemediationConfig) ValidateDelete() (warning admission.Warnings, err error) {
selfNodeRemediationConfigLog.Info("validate delete", "name", r.Name)

return nil
if r.Name == ConfigCRName {
if deploymentNs, err := utils.GetDeploymentNamespace(); err != nil {
selfNodeRemediationConfigLog.Error(err, "validate configuration delete failed", "config name", r.Name)
return admission.Warnings{}, err
} else if deploymentNs == r.Namespace {
return admission.Warnings{"The default configuration is deleted, Self Node Remediation is now disabled"}, nil
}
}
return admission.Warnings{}, nil
}

// validateTimes validates that each time field in the SelfNodeRemediationConfig CR doesn't go below the minimum time
Expand Down
72 changes: 46 additions & 26 deletions api/v1alpha1/selfnoderemediationconfig_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ var _ = Describe("SelfNodeRemediationConfig Validation", func() {

When("CR name doesn't match default name", func() {
It("create should be rejected", func() {
snrc := createDefaultSelfNodeRemediationConfigCR()
err := snrc.ValidateCreate()
snrc := createTestSelfNodeRemediationConfigCR()
_, err := snrc.ValidateCreate()
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("to enforce only one SelfNodeRemediationConfig in the cluster, a name other than"))

Expand All @@ -92,11 +92,11 @@ var _ = Describe("SelfNodeRemediationConfig Validation", func() {

When("CR name namespace does not match deployment namespace", func() {
It("create should be rejected", func() {
snrc := createDefaultSelfNodeRemediationConfigCR()
snrc := createTestSelfNodeRemediationConfigCR()
snrc.Name = ConfigCRName
_ = os.Setenv("DEPLOYMENT_NAMESPACE", "mock-deployment-namespace")

err := snrc.ValidateCreate()
_, err := snrc.ValidateCreate()
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("SelfNodeRemediationConfig is only allowed to be created in the namespace:"))

Expand All @@ -119,6 +119,26 @@ var _ = Describe("SelfNodeRemediationConfig Validation", func() {

})

Describe("deleting SelfNodeRemediationConfig CR", func() {
var conf *SelfNodeRemediationConfig
BeforeEach(func() {
conf = createTestSelfNodeRemediationConfigCR()
})

When("SelfNodeRemediationConfig CR is default", func() {
BeforeEach(func() {
conf.Name = "self-node-remediation-config"
//Mock deployment namespace
_ = os.Setenv("DEPLOYMENT_NAMESPACE", conf.Namespace)
mshitrit marked this conversation as resolved.
Show resolved Hide resolved
})
It("should have warning", func() {
war, err := conf.ValidateDelete()
Expect(err).To(Succeed())
Expect(war[0]).To(ContainSubstring("The default configuration is deleted, Self Node Remediation is now disabled"))
})
})
})

})

func testSingleInvalidField(validationType validationType) {
Expand All @@ -128,14 +148,14 @@ func testSingleInvalidField(validationType validationType) {
text := "for field" + item.name + " with value shorter than " + item.minDurationValue.String()
Context(text, func() {
It("should be rejected", func() {
snrc := createSelfNodeRemediationConfigCR(item.name, item.durationValue)
snrc := createSelfNodeRemediationConfigCRWithFieldValue(item.name, item.durationValue)

var err error
if validationType == update {
snrcOld := createDefaultSelfNodeRemediationConfigCR()
err = snrc.ValidateUpdate(snrcOld)
snrcOld := createTestSelfNodeRemediationConfigCR()
_, err = snrc.ValidateUpdate(snrcOld)
} else {
err = snrc.ValidateCreate()
_, err = snrc.ValidateCreate()
}

Expect(err).To(HaveOccurred())
Expand All @@ -146,30 +166,30 @@ func testSingleInvalidField(validationType validationType) {

Context(fmt.Sprintf("%s validation of customized toleration", validationType.getName()), func() {
It("should be rejected - invalid operator value", func() {
snrc := createDefaultSelfNodeRemediationConfigCR()
snrc := createTestSelfNodeRemediationConfigCR()
snrc.Spec.CustomDsTolerations = []v1.Toleration{{Key: "validValue", Operator: "dummyInvalidOperatorValue"}}

var err error
if validationType == update {
snrcOld := createDefaultSelfNodeRemediationConfigCR()
err = snrc.ValidateUpdate(snrcOld)
snrcOld := createTestSelfNodeRemediationConfigCR()
_, err = snrc.ValidateUpdate(snrcOld)
} else {
err = snrc.ValidateCreate()
_, err = snrc.ValidateCreate()
}

Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("invalid operator for toleration: dummyInvalidOperatorValue"))
})
It("should be rejected- non empty value when operator equals Exists", func() {
snrc := createDefaultSelfNodeRemediationConfigCR()
snrc := createTestSelfNodeRemediationConfigCR()
snrc.Spec.CustomDsTolerations = []v1.Toleration{{Value: "someValue", Operator: "Exists"}}

var err error
if validationType == update {
snrcOld := createDefaultSelfNodeRemediationConfigCR()
err = snrc.ValidateUpdate(snrcOld)
snrcOld := createTestSelfNodeRemediationConfigCR()
_, err = snrc.ValidateUpdate(snrcOld)
} else {
err = snrc.ValidateCreate()
_, err = snrc.ValidateCreate()
}

Expect(err).To(HaveOccurred())
Expand All @@ -180,7 +200,7 @@ func testSingleInvalidField(validationType validationType) {

func testMultipleInvalidFields(validationType validationType) {
var errorMsg string
snrc := createDefaultSelfNodeRemediationConfigCR()
snrc := createTestSelfNodeRemediationConfigCR()

for _, item := range testItems2 {
item := item
Expand All @@ -195,10 +215,10 @@ func testMultipleInvalidFields(validationType validationType) {
It("should be rejected", func() {
var err error
if validationType == update {
snrcOld := createDefaultSelfNodeRemediationConfigCR()
err = snrc.ValidateUpdate(snrcOld)
snrcOld := createTestSelfNodeRemediationConfigCR()
_, err = snrc.ValidateUpdate(snrcOld)
} else {
err = snrc.ValidateCreate()
_, err = snrc.ValidateCreate()
}

Expect(err).To(HaveOccurred())
Expand Down Expand Up @@ -232,18 +252,18 @@ func testValidCR(validationType validationType) {
It("should not be rejected", func() {
var err error
if validationType == update {
snrcOld := createDefaultSelfNodeRemediationConfigCR()
err = snrc.ValidateUpdate(snrcOld)
snrcOld := createTestSelfNodeRemediationConfigCR()
_, err = snrc.ValidateUpdate(snrcOld)
} else {
err = snrc.ValidateCreate()
_, err = snrc.ValidateCreate()
}
Expect(err).NotTo(HaveOccurred())

})
})
}

func createDefaultSelfNodeRemediationConfigCR() *SelfNodeRemediationConfig {
func createTestSelfNodeRemediationConfigCR() *SelfNodeRemediationConfig {
snrc := &SelfNodeRemediationConfig{}
snrc.Name = "test"
snrc.Namespace = "default"
Expand All @@ -259,8 +279,8 @@ func createDefaultSelfNodeRemediationConfigCR() *SelfNodeRemediationConfig {
return snrc
}

func createSelfNodeRemediationConfigCR(fieldName string, value time.Duration) *SelfNodeRemediationConfig {
snrc := createDefaultSelfNodeRemediationConfigCR()
func createSelfNodeRemediationConfigCRWithFieldValue(fieldName string, value time.Duration) *SelfNodeRemediationConfig {
snrc := createTestSelfNodeRemediationConfigCR()

//set the tested field
setFieldValue(snrc, fieldName, value)
Expand Down
13 changes: 7 additions & 6 deletions api/v1alpha1/selfnoderemediationtemplate_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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"

"github.com/medik8s/self-node-remediation/pkg/utils"
)
Expand Down Expand Up @@ -60,22 +61,22 @@ func (r *SelfNodeRemediationTemplate) Default() {
var _ webhook.Validator = &SelfNodeRemediationTemplate{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *SelfNodeRemediationTemplate) ValidateCreate() error {
func (r *SelfNodeRemediationTemplate) ValidateCreate() (warning admission.Warnings, err error) {
webhookTemplateLog.Info("validate create", "name", r.Name)
return validateStrategy(r.Spec.Template.Spec)
return admission.Warnings{}, validateStrategy(r.Spec.Template.Spec)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *SelfNodeRemediationTemplate) ValidateUpdate(_ runtime.Object) error {
func (r *SelfNodeRemediationTemplate) ValidateUpdate(_ runtime.Object) (warning admission.Warnings, err error) {
webhookTemplateLog.Info("validate update", "name", r.Name)
return validateStrategy(r.Spec.Template.Spec)
return admission.Warnings{}, validateStrategy(r.Spec.Template.Spec)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *SelfNodeRemediationTemplate) ValidateDelete() error {
func (r *SelfNodeRemediationTemplate) ValidateDelete() (warning admission.Warnings, err error) {
// unused for now, add "delete" when needed to verbs in the kubebuilder annotation above
webhookTemplateLog.Info("validate delete", "name", r.Name)
return nil
return admission.Warnings{}, nil
}

func validateStrategy(snrSpec SelfNodeRemediationSpec) error {
Expand Down
15 changes: 10 additions & 5 deletions api/v1alpha1/selfnoderemediationtemplate_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ var _ = Describe("SelfNodeRemediationTemplate Validation", func() {

Context("with valid strategy", func() {
It("should be allowed", func() {
Expect(snrtValid.ValidateCreate()).To(Succeed())
_, err := snrtValid.ValidateCreate()
Expect(err).To(Succeed())
})
})

Expand All @@ -61,17 +62,21 @@ var _ = Describe("SelfNodeRemediationTemplate Validation", func() {
utils.IsOutOfServiceTaintSupported = true
})
It("should be allowed", func() {
Expect(outOfServiceStrategy.ValidateCreate()).To(Succeed())
Expect(snrtValid.ValidateUpdate(outOfServiceStrategy)).To(Succeed())
_, err := outOfServiceStrategy.ValidateCreate()
Expect(err).To(Succeed())
_, err = snrtValid.ValidateUpdate(outOfServiceStrategy)
Expect(err).To(Succeed())
})
})
When("out of service taint is not supported", func() {
BeforeEach(func() {
utils.IsOutOfServiceTaintSupported = false
})
It("should be denied", func() {
Expect(outOfServiceStrategy.ValidateCreate()).To(MatchError(ContainSubstring("OutOfServiceTaint remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy")))
Expect(outOfServiceStrategy.ValidateUpdate(snrtValid)).To(MatchError(ContainSubstring("OutOfServiceTaint remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy")))
_, err := outOfServiceStrategy.ValidateCreate()
Expect(err).To(MatchError(ContainSubstring("OutOfServiceTaint remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy")))
_, err = outOfServiceStrategy.ValidateUpdate(snrtValid)
Expect(err).To(MatchError(ContainSubstring("OutOfServiceTaint remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy")))
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ spec:
operations:
- CREATE
- UPDATE
- DELETE
resources:
- selfnoderemediationconfigs
sideEffects: None
Expand Down
Loading
Loading