daemon: skip querying currentConfig if possible

[jlebon]

The same way we pass the specific desiredConfig we "locked on" when
updating, also similarly pass the currentConfig. This is a minor
optimization so we avoid querying the cluster again for information we
already have, but it does also make the code more resilient to humans
meddling with node annotations.

Requires: #310

[cgwalters]

I was looking at a similar patch recently, but I was thinking more like

$ git diff
diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go
index 5309697..c78133a 100644
--- a/pkg/daemon/daemon.go
+++ b/pkg/daemon/daemon.go
@@ -598,7 +598,7 @@ func (dn *Daemon) CheckStateOnBoot() error {
        } else {
                // currentConfig != desiredConfig, and we're not booting up into the desiredConfig.
                // Kick off an update.
-               if err := dn.triggerUpdateWithMachineConfig(stateAndConfigs.desiredConfig); err != nil {
+               if err := dn.update(stateAndConfigs.currentConfig, stateAndConfigs.desiredConfig); err != nil {
                        return err
                }
        }
diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go
index 158f876..4e78920 100644
--- a/pkg/daemon/update.go
+++ b/pkg/daemon/update.go
@@ -69,6 +69,12 @@ func (dn *Daemon) writePendingState(desiredConfig *mcfgv1.MachineConfig) error {
 func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig) error {
        var err error
 
+       if dn.onceFrom != "" && !ValidPath(dn.onceFrom) {
+               if err = dn.nodeWriter.SetUpdateWorking(dn.kubeClient.CoreV1().Nodes(), dn.name); err != nil {
+                       return err
+               }
+       }
+
        oldConfigName := oldConfig.GetName()
        newConfigName := newConfig.GetName()
        glog.Infof("Checking reconcilable for config %v to %v", oldConfigName, newConfigName)

[cgwalters]

There's all these layers of "update/triggerUpdate*" functions that get into the "onceFrom" stuff and I get a bit confused by them...but the lowest level seems to be update() and so let's move the important bit of setting the node annotation there and have the main cluster path call it?

[jlebon]

Yeah, I think that'd work. Though it's a bit more involved than I wanted to go given the current freeze. Whereas the patch as written is pretty easy to reason through. Let's enhance it later? Your diff did make me notice something else though. I pushed another minor commit for that! ⬆️

[ashcrow]

Infra flake

/retest

[ashcrow]

/retest

[ashcrow]

Terraform flake

/retest

[ashcrow]

Deferring to a second review to merge.

[ashcrow]

/approve

[ashcrow]

Monitor flake:

net/http: request canceled (Client.Timeout exceeded while awaiting headers)

/retest

[cgwalters]

/lgtm

This will help a bit for #273

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashcrow, cgwalters, jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• pkg/daemon/OWNERS [ashcrow,cgwalters,jlebon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ashcrow]

Terraform errors with AWS. Will force a test once the clean up occurs.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[ashcrow]

/retest

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.