Merge pull request #55 from euank/agent-bootcycle

agent: wait for operator to reset us on reboot

Author: euank

internal/agent/agent.go

@@ -28,6 +28,13 @@ type Klocksmith struct {
 	lc   *login1.Conn
 }
 
+var (
+	shouldRebootSelector = fields.Set(map[string]string{
+		constants.AnnotationOkToReboot:   constants.True,
+		constants.AnnotationRebootNeeded: constants.True,
+	}).AsSelector()
+)
+
 func New(node string) (*Klocksmith, error) {
 	// set up kubernetes in-cluster client
 	kc, err := k8sutil.InClusterClient()
@@ -127,6 +134,11 @@ func (k *Klocksmith) Run() error {
 	if err := k8sutil.SetNodeAnnotations(k.nc, k.node, anno); err != nil {
 		return err
 	}
+	// Since we set 'reboot-needed=false', 'ok-to-reboot' should clear.
+	// Wait for it to do so, else we might start reboot-looping
+	if err := k.waitForNotOkToReboot(); err != nil {
+		return err
+	}
 
 	// watch update engine for status updates
 	go k.watchUpdateStatus(k.updateStatusCallback)
@@ -235,12 +247,17 @@ func (k *Klocksmith) waitForRebootSignal() error {
 	return nil
 }
 
+// waitForOkToReboot waits for both 'ok-to-reboot' and 'needs-reboot' to be true.
 func (k *Klocksmith) waitForOkToReboot() error {
 	n, err := k.nc.Get(k.node)
 	if err != nil {
 		return fmt.Errorf("failed to get self node (%q): %v", k.node, err)
 	}
 
+	if n.Annotations[constants.AnnotationOkToReboot] == constants.True && n.Annotations[constants.AnnotationRebootNeeded] == constants.True {
+		return nil
+	}
+
 	// XXX: set timeout > 0?
 	watcher, err := k.nc.Watch(v1api.ListOptions{
 		FieldSelector:   fields.OneTermEqualSelector("metadata.name", n.Name).String(),
@@ -252,7 +269,7 @@ func (k *Klocksmith) waitForOkToReboot() error {
 
 	// hopefully 24 hours is enough time between indicating we need a
 	// reboot and the controller telling us to do it
-	ev, err := watch.Until(time.Hour*24, watcher, k8sutil.NodeAnnotationCondition(constants.AnnotationOkToReboot, constants.True))
+	ev, err := watch.Until(time.Hour*24, watcher, k8sutil.NodeAnnotationCondition(shouldRebootSelector))
 	if err != nil {
 		return fmt.Errorf("waiting for annotation %q failed: %v", constants.AnnotationOkToReboot, err)
 	}
@@ -270,6 +287,60 @@ func (k *Klocksmith) waitForOkToReboot() error {
 	return nil
 }
 
+func (k *Klocksmith) waitForNotOkToReboot() error {
+	n, err := k.nc.Get(k.node)
+	if err != nil {
+		return fmt.Errorf("failed to get self node (%q): %v", k.node, err)
+	}
+
+	if n.Annotations[constants.AnnotationOkToReboot] != constants.True {
+		return nil
+	}
+
+	// XXX: set timeout > 0?
+	watcher, err := k.nc.Watch(v1api.ListOptions{
+		FieldSelector:   fields.OneTermEqualSelector("metadata.name", n.Name).String(),
+		ResourceVersion: n.ResourceVersion,
+	})
+	if err != nil {
+		return fmt.Errorf("failed to watch self node (%q): %v", k.node, err)
+	}
+
+	// Within 24 hours of indicating we don't need a reboot we should be given a not-ok.
+	// If that isn't the case, it likely means the operator isn't running, and
+	// we'll just crash-loop in that case, and hopefully that will help the user realize something's wrong.
+	// Use a custom condition function to use the more correct 'OkToReboot !=
+	// true' vs '== False'; due to the operator matching on '== True', and not
+	// going out of its way to convert '' => 'False', checking the exact inverse
+	// of what the operator checks is the correct thing to do.
+	ev, err := watch.Until(time.Hour*24, watcher, watch.ConditionFunc(func(event watch.Event) (bool, error) {
+		switch event.Type {
+		case watch.Error:
+			return false, fmt.Errorf("error watching node: %v", event.Object)
+		case watch.Deleted:
+			return false, fmt.Errorf("our node was deleted while we were waiting for ready")
+		}
+
+		no := event.Object.(*v1api.Node)
+		if no.Annotations[constants.AnnotationOkToReboot] != constants.True {
+			return true, nil
+		}
+		return false, nil
+	}))
+	if err != nil {
+		return fmt.Errorf("waiting for annotation %q failed: %v", constants.AnnotationOkToReboot, err)
+	}
+
+	// sanity check
+	no := ev.Object.(*v1api.Node)
+
+	if no.Annotations[constants.AnnotationOkToReboot] == constants.True {
+		panic("event did not contain annotation expected")
+	}
+
+	return nil
+}
+
 func (k *Klocksmith) getPodsForDeletion() ([]v1api.Pod, error) {
 	pods, err := drain.GetPodsForDeletion(k.kc, k.node)
 	if err != nil {

internal/k8sutil/metadata.go

@@ -10,6 +10,7 @@ import (
 	"github.com/golang/glog"
 	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
 	v1api "k8s.io/client-go/pkg/api/v1"
+	"k8s.io/client-go/pkg/fields"
 	"k8s.io/client-go/pkg/watch"
 )
 
@@ -21,16 +22,12 @@ const (
 
 // NodeAnnotationCondition returns a condition function that succeeds when a
 // node being watched has an annotation of key equal to value.
-func NodeAnnotationCondition(key, value string) watch.ConditionFunc {
+func NodeAnnotationCondition(selector fields.Selector) watch.ConditionFunc {
 	return func(event watch.Event) (bool, error) {
 		switch event.Type {
 		case watch.Modified:
 			node := event.Object.(*v1api.Node)
-			if node.Annotations[key] == value {
-				return true, nil
-			}
-
-			return false, nil
+			return selector.Matches(fields.Set(node.Annotations)), nil
 		}
 
 		return false, fmt.Errorf("unhandled watch case for %#v", event)

internal/operator/operator.go

@@ -268,15 +268,11 @@ func (k *Kontroller) Run(ctx context.Context) error {
 func (k *Kontroller) markNodeRebootable(n *v1api.Node) {
 	glog.V(4).Infof("marking %s ok to reboot", n.Name)
 	if err := k8sutil.UpdateNodeRetry(k.nc, n.Name, func(node *v1api.Node) {
-		// TODO; reuse selector if I can figure out how to apply it to a single node
-		if node.Annotations[constants.AnnotationOkToReboot] == constants.True {
-			glog.Warningf("Node %v became rebootable while we were trying to mark it so", node.Name)
-			return
-		}
-		if node.Annotations[constants.AnnotationRebootNeeded] != constants.True {
-			glog.Warningf("Node %v became not-ok-for-reboot while trying to mark it ready", node.Name)
+		if !wantsRebootSelector.Matches(fields.Set(node.Annotations)) {
+			glog.Warningf("Node %v no longer wanted to a reboot while we were trying to mark it so: %v", node.Name, node.Annotations)
 			return
 		}
+
 		node.Annotations[constants.AnnotationOkToReboot] = constants.True
 	}); err != nil {
 		glog.Infof("Failed to set annotation %q on node %q: %v", constants.AnnotationOkToReboot, n.Name, err)