From da9c784dd6b0a1ff5fbf484ac6611e797e2d8137 Mon Sep 17 00:00:00 2001 From: Nuckal777 Date: Wed, 7 Aug 2024 11:28:43 +0200 Subject: [PATCH] Add state enter trigger chain --- controllers/config.go | 8 ++++++++ controllers/node_controller_test.go | 3 +++ controllers/node_handler.go | 5 +---- controllers/suite_test.go | 7 +++++++ docs/concepts.md | 18 ++++++++++++++++++ docs/configuration.md | 2 +- docs/operations.md | 4 ++-- metrics/metrics.go | 4 ++++ state/maintenance.go | 3 +++ state/maintenance_test.go | 10 ++++++++++ state/operational.go | 2 +- state/operational_test.go | 9 +++++++++ state/required.go | 2 +- state/required_test.go | 10 ++++++++++ state/state.go | 1 + state/state_test.go | 16 ++++++++++++++++ 16 files changed, 95 insertions(+), 9 deletions(-) diff --git a/controllers/config.go b/controllers/config.go index 4ecc9fb..a21f439 100644 --- a/controllers/config.go +++ b/controllers/config.go @@ -38,6 +38,7 @@ type ProfileDescriptor struct { } type StateDescriptor struct { + Enter string Notify string Transitions []TransitionDescriptor } @@ -144,6 +145,13 @@ func loadPluginChains(config StateDescriptor, registry *plugin.Registry) (state. return chains, err } chains.Notification = notificationChain + + enterChain, err := registry.NewTriggerChain(config.Enter) + if err != nil { + return chains, err + } + chains.Enter = enterChain + chains.Transitions = make([]state.Transition, 0) for _, transitionConfig := range config.Transitions { var transition state.Transition diff --git a/controllers/node_controller_test.go b/controllers/node_controller_test.go index e6eebb2..d934dc8 100644 --- a/controllers/node_controller_test.go +++ b/controllers/node_controller_test.go @@ -253,14 +253,17 @@ var _ = Describe("The controller", func() { Expect(operational.Transitions[0].Check.Plugins).To(HaveLen(1)) Expect(operational.Notification.Plugins).To(BeEmpty()) Expect(operational.Transitions[0].Trigger.Plugins).To(HaveLen(1)) + Expect(operational.Enter.Plugins).To(BeEmpty()) required := profile.Chains[state.Required] Expect(required.Transitions[0].Check.Plugins).To(HaveLen(2)) Expect(required.Notification.Plugins).To(BeEmpty()) Expect(required.Transitions[0].Trigger.Plugins).To(BeEmpty()) + Expect(required.Enter.Plugins).To(BeEmpty()) maintenance := profile.Chains[state.InMaintenance] Expect(maintenance.Transitions[0].Check.Plugins).To(HaveLen(3)) Expect(maintenance.Notification.Plugins).To(BeEmpty()) Expect(maintenance.Transitions[0].Trigger.Plugins).To(BeEmpty()) + Expect(maintenance.Enter.Plugins).To(HaveLen(1)) }) }) diff --git a/controllers/node_handler.go b/controllers/node_handler.go index ff80fe8..a562947 100644 --- a/controllers/node_handler.go +++ b/controllers/node_handler.go @@ -80,10 +80,7 @@ func ApplyProfiles(ctx context.Context, params reconcileParameters, data *state. continue } - logDetails := false - if params.node.Labels[constants.LogDetailsLabelKey] == "true" { - logDetails = true - } + logDetails := params.node.Labels[constants.LogDetailsLabelKey] == "true" // build plugin arguments pluginParams := plugin.Parameters{Client: params.client, Clientset: params.clientset, Ctx: ctx, Log: params.log, Profile: ps.Profile.Name, Node: params.node, InMaintenance: anyInMaintenance(profileStates), diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 210c8b9..586ad55 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -72,6 +72,12 @@ instances: key: alter value: "true" remove: false + - type: alterLabel + name: entered + config: + key: entered + value: "true" + remove: false profiles: - name: count operational: @@ -84,6 +90,7 @@ profiles: - check: transition && transition next: in-maintenance in-maintenance: + enter: entered transitions: - check: transition && transition && transition next: operational diff --git a/docs/concepts.md b/docs/concepts.md index 4ef252f..49b0673 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -84,3 +84,21 @@ notify: config: interval: 24h ``` + +## Triggers on entering a state +Each state in a maintenance profile can optionally specify a list of trigger plugin instances, which are executed when the profile enters that state. +This is mostly useful in conjunction with the `in-maintenance` state and the `maxMaintenance` and `eviction` plugins to drain nodes via the maintenance-controller. +Draining a node as part of a transition is prone to race conditions, when the maintenance-controller drains itself and a different node could move into the `in-maintenance` state. + +```yaml +maintenance-required: + transitions: + - check: nodes_in_maintenance + next: in-maintenance +in-maintenance: + enter: drain_node + transitions: + - check: node_ready + trigger: remove_approval + next: operational +``` diff --git a/docs/configuration.md b/docs/configuration.md index aa3fa33..a783c74 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -60,8 +60,8 @@ Each state has a name, a list of transitions, and a list of notification instanc ```yaml profiles: - name: os-patching - notify: notify_slack operational: + notify: notify_slack transitions: - check: check_approval trigger: remove_approval diff --git a/docs/operations.md b/docs/operations.md index 69e2d29..40b9b04 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -19,5 +19,5 @@ The maintenance-controller creates Kubernetes events on nodes for each state tra These are visible in the `kubectl describe node` as well as `kubectl get events` output. Also, the maintenance-controller logs all errors and informational messages to the standard output. -When multiple instances of the maintenance-controller are running in a cluster, the `--enable-leader-election` must be set. -Otherwise, the instances might interfere with each other. +When multiple instances of the maintenance-controller are running in a cluster, the `--enable-leader-election` **must** be set. +Otherwise, the instances will interfere with each other. diff --git a/metrics/metrics.go b/metrics/metrics.go index c9cbf1d..2116921 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -21,6 +21,7 @@ package metrics import ( "context" + "errors" "fmt" "github.com/prometheus/client_golang/prometheus" @@ -68,6 +69,9 @@ type fetchParams struct { // Actually increment shuffle counters. func RecordShuffles(ctx context.Context, k8sClient client.Client, node *v1.Node, currentProfile string) error { var podList v1.PodList + if k8sClient == nil { + return errors.New("kubernetes client is nil") + } err := k8sClient.List(ctx, &podList, client.MatchingFields{"spec.nodeName": node.Name}) if err != nil { return err diff --git a/state/maintenance.go b/state/maintenance.go index bbb175b..e34e77a 100644 --- a/state/maintenance.go +++ b/state/maintenance.go @@ -41,6 +41,9 @@ func (s *inMaintenance) Label() NodeStateLabel { } func (s *inMaintenance) Enter(params plugin.Parameters, data *DataV2) error { + if err := s.chains.Enter.Execute(params); err != nil { + return err + } if err := metrics.RecordShuffles( params.Ctx, params.Client, diff --git a/state/maintenance_test.go b/state/maintenance_test.go index 9aa5344..ecd6953 100644 --- a/state/maintenance_test.go +++ b/state/maintenance_test.go @@ -129,5 +129,15 @@ var _ = Describe("InMaintenance State", func() { Expect(result.Infos[0].Error).ToNot(BeEmpty()) Expect(check.Invoked).To(Equal(1)) }) + + It("executes the enter chain", func() { + chain, enter := mockTriggerChain() + chains.Enter = chain + im := newInMaintenance(chains) + err := im.Enter(plugin.Parameters{Log: GinkgoLogr}, &DataV2{}) + Expect(err).To(Succeed()) + Expect(enter.Invoked).To(Equal(1)) + }) + }) }) diff --git a/state/operational.go b/state/operational.go index 76722fd..d1a0268 100644 --- a/state/operational.go +++ b/state/operational.go @@ -41,7 +41,7 @@ func (s *operational) Label() NodeStateLabel { } func (s *operational) Enter(params plugin.Parameters, data *DataV2) error { - return nil + return s.chains.Enter.Execute(params) } func (s *operational) Notify(params plugin.Parameters, data *DataV2) error { diff --git a/state/operational_test.go b/state/operational_test.go index 4348823..19c46ec 100644 --- a/state/operational_test.go +++ b/state/operational_test.go @@ -131,6 +131,15 @@ var _ = Describe("Operational State", func() { Expect(check.Invoked).To(Equal(1)) }) + It("executes the enter chain", func() { + chain, enter := mockTriggerChain() + chains.Enter = chain + op := newOperational(chains) + err := op.Enter(plugin.Parameters{Log: GinkgoLogr}, &DataV2{}) + Expect(err).To(Succeed()) + Expect(enter.Invoked).To(Equal(1)) + }) + }) It("should execute the notification chain if the state has changed", func() { diff --git a/state/required.go b/state/required.go index 0162a5f..53ed166 100644 --- a/state/required.go +++ b/state/required.go @@ -41,7 +41,7 @@ func (s *maintenanceRequired) Label() NodeStateLabel { } func (s *maintenanceRequired) Enter(params plugin.Parameters, data *DataV2) error { - return nil + return s.chains.Enter.Execute(params) } func (s *maintenanceRequired) Notify(params plugin.Parameters, data *DataV2) error { diff --git a/state/required_test.go b/state/required_test.go index 3cda4bd..f1b9dcc 100644 --- a/state/required_test.go +++ b/state/required_test.go @@ -135,5 +135,15 @@ var _ = Describe("MaintenanceRequired State", func() { Expect(result.Infos[0].Error).ToNot(BeEmpty()) Expect(check.Invoked).To(Equal(1)) }) + + It("executes the enter chain", func() { + chain, enter := mockTriggerChain() + chains.Enter = chain + mr := newOperational(chains) + err := mr.Enter(plugin.Parameters{Log: GinkgoLogr}, &DataV2{}) + Expect(err).To(Succeed()) + Expect(enter.Invoked).To(Equal(1)) + }) + }) }) diff --git a/state/state.go b/state/state.go index 7cf1f98..0bce72c 100644 --- a/state/state.go +++ b/state/state.go @@ -111,6 +111,7 @@ type NodeInfo struct { // PluginChains is a struct containing a plugin chain of each plugin type. type PluginChains struct { + Enter plugin.TriggerChain Notification plugin.NotificationChain Transitions []Transition } diff --git a/state/state_test.go b/state/state_test.go index 5b6f158..79de4a4 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -294,6 +294,22 @@ var _ = Describe("Apply", func() { Expect(result.Error).ToNot(BeEmpty()) }) + It("invokes Enter() when the previous state is different from the current state", func() { + chain, enter := mockTriggerChain() + nodeState := operational{ + label: Operational, + chains: PluginChains{ + Enter: chain, + }, + } + data := DataV2{Profiles: map[string]*ProfileData{"profile": {Current: Operational, Previous: InMaintenance}}} + result, err := Apply(&nodeState, &v1.Node{}, &data, buildParams()) + Expect(err).To(Succeed()) + Expect(result.Next).To(Equal(Operational)) + Expect(result.Transitions).To(BeEmpty()) + Expect(enter.Invoked).To(Equal(1)) + }) + }) var _ = Describe("ParseData", func() {