Add state enter trigger chain

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

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))
 	})
 
 })

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),

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

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
+```

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

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.

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

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,

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))
+		})
+
 	})
 })

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 {

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() {

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 {

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))
+		})
+
 	})
 })

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
 }

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() {