Skip to content

Commit

Permalink
fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@sbueringer
sbueringer committed Oct 31, 2024
1 parent 6b5e56e commit b8a7e87
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 9 deletions.
6 changes: 4 additions & 2 deletions api/v1beta1/machinedrainrules_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ type MachineDrainRuleDrainConfig struct {
Order *int32 `json:"order,omitempty"`
}

// MachineDrainRuleMachineSelector defines to which Machines this MachineDrainRule should be applied.
// +kubebuilder:validation:MinProperties=1
type MachineDrainRuleMachineSelector struct { //nolint:revive // Intentionally not adding a godoc comment as it would show up additionally to the field comment in the CRD
type MachineDrainRuleMachineSelector struct {
// selector is a label selector which selects Machines by their labels.
// This field follows standard label selector semantics; if not present or
// empty, it selects all Machines.
Expand All @@ -160,8 +161,9 @@ type MachineDrainRuleMachineSelector struct { //nolint:revive // Intentionally n
ClusterSelector *metav1.LabelSelector `json:"clusterSelector,omitempty"`
}

// MachineDrainRulePodSelector defines to which Pods this MachineDrainRule should be applied.
// +kubebuilder:validation:MinProperties=1
type MachineDrainRulePodSelector struct { //nolint:revive // Intentionally not adding a godoc comment as it would show up additionally to the field comment in the CRD
type MachineDrainRulePodSelector struct {
// selector is a label selector which selects Pods by their labels.
// This field follows standard label selector semantics; if not present or
// empty, it selects all Pods.
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/cluster.x-k8s.io_machinedrainrules.yaml
View file

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

3 changes: 3 additions & 0 deletions internal/test/envtest/environment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,9 @@ func newEnvironment(uncachedObjs ...client.Object) *Environment {
if err := (&webhooks.MachineDeployment{}).SetupWebhookWithManager(mgr); err != nil {
klog.Fatalf("unable to create webhook: %+v", err)
}
if err := (&webhooks.MachineDrainRule{}).SetupWebhookWithManager(mgr); err != nil {
klog.Fatalf("unable to create webhook: %+v", err)
}
if err := (&bootstrapwebhooks.KubeadmConfig{}).SetupWebhookWithManager(mgr); err != nil {
klog.Fatalf("unable to create webhook: %+v", err)
}
Expand Down
90 changes: 83 additions & 7 deletions internal/webhooks/machinedrainrules_test.go → ...l/webhooks/test/machinedrainrules_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package webhooks
package test

import (
"testing"
Expand Down Expand Up @@ -44,7 +44,8 @@ func Test_validate(t *testing.T) {
name: "Return no error if MachineDrainRule is valid",
machineDrainRule: &clusterv1.MachineDrainRule{
ObjectMeta: metav1.ObjectMeta{
Name: "mdr",
Name: "mdr",
Namespace: metav1.NamespaceDefault,
},
Spec: clusterv1.MachineDrainRuleSpec{
Drain: clusterv1.MachineDrainRuleDrainConfig{
Expand Down Expand Up @@ -86,7 +87,8 @@ func Test_validate(t *testing.T) {
name: "Return error if order is set with drain behavior Skip",
machineDrainRule: &clusterv1.MachineDrainRule{
ObjectMeta: metav1.ObjectMeta{
Name: "mdr",
Name: "mdr",
Namespace: metav1.NamespaceDefault,
},
Spec: clusterv1.MachineDrainRuleSpec{
Drain: clusterv1.MachineDrainRuleDrainConfig{
Expand All @@ -95,14 +97,16 @@ func Test_validate(t *testing.T) {
},
},
},
wantErr: "MachineDrainRule.cluster.x-k8s.io \"mdr\" is invalid: " +
wantErr: "admission webhook \"validation.machinedrainrule.cluster.x-k8s.io\" denied the request: " +
"MachineDrainRule.cluster.x-k8s.io \"mdr\" is invalid: " +
"spec.drain.order: Invalid value: 5: order must not be set if drain behavior is \"Skip\"",
},
{
name: "Return error for MachineDrainRules with invalid selector",
machineDrainRule: &clusterv1.MachineDrainRule{
ObjectMeta: metav1.ObjectMeta{
Name: "mdr",
Name: "mdr",
Namespace: metav1.NamespaceDefault,
},
Spec: clusterv1.MachineDrainRuleSpec{
Drain: clusterv1.MachineDrainRuleDrainConfig{
Expand All @@ -122,24 +126,96 @@ func Test_validate(t *testing.T) {
},
},
},
wantErr: "MachineDrainRule.cluster.x-k8s.io \"mdr\" is invalid: [" +
wantErr: "admission webhook \"validation.machinedrainrule.cluster.x-k8s.io\" denied the request: " +
"MachineDrainRule.cluster.x-k8s.io \"mdr\" is invalid: [" +
"spec.machines[0].selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string(nil), MatchExpressions:[]v1.LabelSelectorRequirement{v1.LabelSelectorRequirement{Key:\"\", Operator:\"Invalid-Operator\", Values:[]string(nil)}}}: \"Invalid-Operator\" is not a valid label selector operator, " +
"spec.machines[0].clusterSelector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string(nil), MatchExpressions:[]v1.LabelSelectorRequirement{v1.LabelSelectorRequirement{Key:\"\", Operator:\"Invalid-Operator\", Values:[]string(nil)}}}: \"Invalid-Operator\" is not a valid label selector operator, " +
"spec.pods[0].selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string(nil), MatchExpressions:[]v1.LabelSelectorRequirement{v1.LabelSelectorRequirement{Key:\"\", Operator:\"Invalid-Operator\", Values:[]string(nil)}}}: \"Invalid-Operator\" is not a valid label selector operator, " +
"spec.pods[0].namespaceSelector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string(nil), MatchExpressions:[]v1.LabelSelectorRequirement{v1.LabelSelectorRequirement{Key:\"\", Operator:\"Invalid-Operator\", Values:[]string(nil)}}}: \"Invalid-Operator\" is not a valid label selector operator]",
},
{
name: "Return error if selectors are not unique",
machineDrainRule: &clusterv1.MachineDrainRule{
ObjectMeta: metav1.ObjectMeta{
Name: "mdr",
Namespace: metav1.NamespaceDefault,
},
Spec: clusterv1.MachineDrainRuleSpec{
Drain: clusterv1.MachineDrainRuleDrainConfig{
Behavior: clusterv1.MachineDrainRuleDrainBehaviorSkip,
},
Machines: []clusterv1.MachineDrainRuleMachineSelector{
{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"os": "linux",
},
},
ClusterSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"stage": "production",
},
},
},
{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"os": "linux",
},
},
ClusterSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"stage": "production",
},
},
},
},
Pods: []clusterv1.MachineDrainRulePodSelector{
{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "does-not-match",
},
},
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"kubernetes.io/metadata.name": "monitoring",
},
},
},
{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "does-not-match",
},
},
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"kubernetes.io/metadata.name": "monitoring",
},
},
},
},
},
},
wantErr: "MachineDrainRule.cluster.x-k8s.io \"mdr\" is invalid: [" +
"spec.machines: Invalid value: \"array\": entries in machines must be unique, " +
"spec.pods: Invalid value: \"array\": entries in pods must be unique]",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

err := (&MachineDrainRule{}).validate(tt.machineDrainRule)
err := env.CreateAndWait(ctx, tt.machineDrainRule)

if tt.wantErr != "" {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(BeComparableTo(tt.wantErr))
} else {
g.Expect(err).ToNot(HaveOccurred())
g.Expect(env.CleanupAndWait(ctx, tt.machineDrainRule)).To(Succeed())
}
})
}
Expand Down

0 comments on commit b8a7e87

Please sign in to comment.