Skip to content

Commit

Permalink
fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
sbueringer committed Oct 31, 2024
1 parent ebc03fc commit 6b5e56e
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 48 deletions.
31 changes: 17 additions & 14 deletions api/v1beta1/machinedrainrules_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,13 @@ type MachineDrainRuleSpec struct {

// machines defines to which Machines this MachineDrainRule should be applied.
//
// If machines is empty, the MachineDrainRule applies to all Machines in the Namespace.
// If machines is not set, the MachineDrainRule applies to all Machines in the Namespace.
// If machines contains multiple selectors, the results are ORed.
// Within a single Machine selector the results of selector and clusterSelector are ANDed.
// Machines will be selected from all Clusters in the Namespace unless otherwise
// restricted with the clusterSelector.
// An empty Machine selector matches all Machines in the Namespace.
// This field is required.
//
// Example: Selects control plane Machines in all Clusters and
// Example: Selects control plane Machines in all Clusters or
// Machines with label "os" == "linux" in Clusters with label
// "stage" == "production".
//
Expand All @@ -73,22 +71,22 @@ type MachineDrainRuleSpec struct {
// values:
// - production
//
// +required
// +optional
// +listType=atomic
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=32
Machines []MachineDrainRuleMachineSelector `json:"machines"`
// +kubebuilder:validation:XValidation:rule="self.all(x, self.exists_one(y, x == y))",message="entries in machines must be unique"
Machines []MachineDrainRuleMachineSelector `json:"machines,omitempty"`

// pods defines to which Pods this MachineDrainRule should be applied.
//
// If pods is empty, the MachineDrainRule applies to all Pods in all Namespaces.
// If pods is not set, the MachineDrainRule applies to all Pods in all Namespaces.
// If pods contains multiple selectors, the results are ORed.
// Within a single Pod selector the results of selector and namespaceSelector are ANDed.
// Pods will be selected from all Namespaces unless otherwise
// restricted with the namespaceSelector.
// An empty Pod selector matches all Pods in all Namespaces.
// This field is required.
//
// Example: Selects Pods with label "app" == "logging" in all Namespaces and
// Example: Selects Pods with label "app" == "logging" in all Namespaces or
// Pods with label "app" == "prometheus" in the "monitoring"
// Namespace.
//
Expand All @@ -105,10 +103,12 @@ type MachineDrainRuleSpec struct {
// matchLabels:
// kubernetes.io/metadata.name: monitoring
//
// +required
// +optional
// +listType=atomic
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=32
Pods []MachineDrainRulePodSelector `json:"pods"`
// +kubebuilder:validation:XValidation:rule="self.all(x, self.exists_one(y, x == y))",message="entries in pods must be unique"
Pods []MachineDrainRulePodSelector `json:"pods,omitempty"`
}

// MachineDrainRuleDrainConfig configures if and how Pods are drained.
Expand All @@ -134,6 +134,7 @@ type MachineDrainRuleDrainConfig struct {
Order *int32 `json:"order,omitempty"`
}

// +kubebuilder:validation:MinProperties=1

Check failure on line 137 in api/v1beta1/machinedrainrules_types.go

View workflow job for this annotation

GitHub Actions / lint

exported: comment on exported type MachineDrainRuleMachineSelector should be of the form "MachineDrainRuleMachineSelector ..." (with optional leading article) (revive)
type MachineDrainRuleMachineSelector struct { //nolint:revive // Intentionally not adding a godoc comment as it would show up additionally to the field comment in the CRD

Check failure on line 138 in api/v1beta1/machinedrainrules_types.go

View workflow job for this annotation

GitHub Actions / lint

directive `//nolint:revive // Intentionally not adding a godoc comment as it would show up additionally to the field comment in the CRD` is unused for linter "revive" (nolintlint)
// selector is a label selector which selects Machines by their labels.
// This field follows standard label selector semantics; if not present or
Expand All @@ -159,6 +160,7 @@ type MachineDrainRuleMachineSelector struct { //nolint:revive // Intentionally n
ClusterSelector *metav1.LabelSelector `json:"clusterSelector,omitempty"`
}

// +kubebuilder:validation:MinProperties=1

Check failure on line 163 in api/v1beta1/machinedrainrules_types.go

View workflow job for this annotation

GitHub Actions / lint

exported: comment on exported type MachineDrainRulePodSelector should be of the form "MachineDrainRulePodSelector ..." (with optional leading article) (revive)
type MachineDrainRulePodSelector struct { //nolint:revive // Intentionally not adding a godoc comment as it would show up additionally to the field comment in the CRD

Check failure on line 164 in api/v1beta1/machinedrainrules_types.go

View workflow job for this annotation

GitHub Actions / lint

directive `//nolint:revive // Intentionally not adding a godoc comment as it would show up additionally to the field comment in the CRD` is unused for linter "revive" (nolintlint)
// selector is a label selector which selects Pods by their labels.
// This field follows standard label selector semantics; if not present or
Expand All @@ -185,7 +187,7 @@ type MachineDrainRulePodSelector struct { //nolint:revive // Intentionally not a
}

// +kubebuilder:object:root=true
// +kubebuilder:resource:path=machinedrainrules,shortName=mdr,scope=Namespaced,categories=cluster-api
// +kubebuilder:resource:path=machinedrainrules,scope=Namespaced,categories=cluster-api
// +kubebuilder:storageversion
// +kubebuilder:printcolumn:name="Behavior",type="string",JSONPath=".spec.drain.behavior",description="Drain behavior"
// +kubebuilder:printcolumn:name="Order",type="string",JSONPath=".spec.drain.order",description="Drain order"
Expand All @@ -196,7 +198,8 @@ type MachineDrainRule struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec MachineDrainRuleSpec `json:"spec,omitempty"`
// +required
Spec MachineDrainRuleSpec `json:"spec"`
}

// +kubebuilder:object:root=true
Expand Down
28 changes: 16 additions & 12 deletions config/crd/bases/cluster.x-k8s.io_machinedrainrules.yaml

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

2 changes: 1 addition & 1 deletion docs/book/src/reference/api/labels-and-annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
| cluster.x-k8s.io/control-plane | It is set on machines or related objects that are part of a control plane. | Cluster API | Machines |
| cluster.x-k8s.io/control-plane-name | It is set on machines if they're controlled by a control plane. The value of this label may be a hash if the control plane name is longer than 63 characters. | Cluster API | Machines |
| cluster.x-k8s.io/deployment-name | It is set on machines if they're controlled by a MachineDeployment. | Cluster API | Machines |
| cluster.x-k8s.io/drain | If set with the value "skip" on a Pod in the workload cluster, the Pod will not be evicted during Node drain. | User | Pods (workload cluster) |
| cluster.x-k8s.io/interruptible | It is used to mark the nodes that run on interruptible instances. | User | Nodes (workload cluster) |
| cluster.x-k8s.io/pool-name | It is set on machines if they're controlled by a MachinePool. | Cluster API | Machines |
| cluster.x-k8s.io/provider | It is set on components in the provider manifest. The label allows one to easily identify all the components belonging to a provider. The clusterctl tool uses this label for implementing provider's lifecycle operations. | User | Provider Components |
Expand All @@ -14,7 +15,6 @@
| machine-template-hash | It is applied to Machines in a MachineDeployment containing the hash of the template. | Cluster API | Machines |
| topology.cluster.x-k8s.io/deployment-name | It is set on the generated MachineDeployment objects to track the name of the MachineDeployment topology it represents. | Cluster API | MachineDeployments |
| topology.cluster.x-k8s.io/owned | It is set on all the object which are managed as part of a ClusterTopology. | Cluster API | ClusterTopology objects |
| cluster.x-k8s.io/drain | If set with the value "skip" on a Pod in the workload cluster, the Pod will not be evicted during Node drain. | User | Pods (workload cluster) |

# Supported Annotations

Expand Down
21 changes: 21 additions & 0 deletions internal/controllers/machine/drain/drain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,15 @@ func TestGetPodsForEviction(t *testing.T) {
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "pod-2-skip-pod-with-drain-label",
Namespace: metav1.NamespaceDefault,
Labels: map[string]string{
clusterv1.PodDrainLabel: "Skip",
},
},
},
},
wantPodDeleteList: PodDeleteList{items: []PodDelete{
{
Expand All @@ -642,6 +651,18 @@ func TestGetPodsForEviction(t *testing.T) {
Reason: PodDeleteStatusTypeSkip,
},
},
{
Pod: &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod-2-skip-pod-with-drain-label",
Namespace: metav1.NamespaceDefault,
},
},
Status: PodDeleteStatus{
DrainBehavior: clusterv1.MachineDrainRuleDrainBehaviorSkip,
Reason: PodDeleteStatusTypeSkip,
},
},
}},
},
{
Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/machine/drain/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func (d *Helper) skipDeletedFilter(_ context.Context, pod *corev1.Pod) PodDelete
}

func (d *Helper) drainLabelFilter(_ context.Context, pod *corev1.Pod) PodDeleteStatus {
if labelValue, found := pod.ObjectMeta.Labels[clusterv1.PodDrainLabel]; found && labelValue == "skip" {
if labelValue, found := pod.ObjectMeta.Labels[clusterv1.PodDrainLabel]; found && strings.EqualFold(labelValue, string(clusterv1.MachineDrainRuleDrainBehaviorSkip)) {
return MakePodDeleteStatusSkip()
}
return MakePodDeleteStatusOkay()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"sync"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -155,7 +157,7 @@ func (r *InMemoryClusterReconciler) reconcileHotRestart(ctx context.Context) err
return nil
}

func (r *InMemoryClusterReconciler) reconcileNormal(_ context.Context, cluster *clusterv1.Cluster, inMemoryCluster *infrav1.InMemoryCluster) error {
func (r *InMemoryClusterReconciler) reconcileNormal(ctx context.Context, cluster *clusterv1.Cluster, inMemoryCluster *infrav1.InMemoryCluster) error {
// Compute the name for resource group and listener.
// NOTE: we are using the same name for convenience, but it is not required.
resourceGroup := klog.KObj(cluster).String()
Expand All @@ -170,6 +172,30 @@ func (r *InMemoryClusterReconciler) reconcileNormal(_ context.Context, cluster *
// well as Kubernetes resources that are expected to exist on the workload cluster (e.g Nodes).
r.InMemoryManager.AddResourceGroup(resourceGroup)

inmemoryClient := r.InMemoryManager.GetResourceGroup(resourceGroup).GetClient()

// Create default Namespaces.
for _, nsName := range []string{metav1.NamespaceDefault, metav1.NamespacePublic, metav1.NamespaceSystem} {
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: nsName,
Labels: map[string]string{
"kubernetes.io/metadata.name": nsName,
},
},
}

if err := inmemoryClient.Get(ctx, client.ObjectKeyFromObject(ns), ns); err != nil {
if !apierrors.IsNotFound(err) {
return errors.Wrapf(err, "failed to get %s Namespace", nsName)
}

if err := inmemoryClient.Create(ctx, ns); err != nil && !apierrors.IsAlreadyExists(err) {
return errors.Wrapf(err, "failed to create %s Namespace", nsName)
}
}
}

// Initialize a listener for the workload cluster; if the listener has been already initialized
// the operation is a no-op.
listener, err := r.APIServerMux.InitWorkloadClusterListener(listenerName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -663,25 +663,6 @@ func (r *InMemoryMachineReconciler) reconcileNormalAPIServer(ctx context.Context
}
}

// Create kube-system namespace
kubeSystemNS := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: metav1.NamespaceSystem,
Labels: map[string]string{
"kubernetes.io/metadata.name": metav1.NamespaceSystem,
},
},
}
if err := inmemoryClient.Get(ctx, client.ObjectKeyFromObject(kubeSystemNS), kubeSystemNS); err != nil {
if !apierrors.IsNotFound(err) {
return ctrl.Result{}, errors.Wrapf(err, "failed to get kube-system Namespace")
}

if err := inmemoryClient.Create(ctx, kubeSystemNS); err != nil && !apierrors.IsAlreadyExists(err) {
return ctrl.Result{}, errors.Wrapf(err, "failed to create kube-system Namespace")
}
}

// If there is not yet an API server listener for this machine.
if !r.APIServerMux.HasAPIServer(listenerName, apiServer) {
// Getting the Kubernetes CA
Expand Down

0 comments on commit 6b5e56e

Please sign in to comment.