Separate out a fi.InstallContext #14814
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
area/nodeup
size/L
johngmyers
changed the title
k8s-ci-robot
added
the
do-not-merge/work-in-progress
| Tasks: tasks, | ||
| } | ||
| i.Build(buildContext) | ||
|
|
||
| // If there is a package task, we need an update packages task |
There was a problem hiding this comment.
I guess the idea is that we know there isn't a Package task? That makes sense if so!
There was a problem hiding this comment.
There's only a File and a Service.
| @@ -165,16 +146,16 @@ func (i *Installation) buildEnvFile() *nodetasks.File { | |||
| sysconfig += key + "=" + value + "\n" | |||
| } | |||
|
|
|||
| task := &nodetasks.File{ | |||
| task := &nodetasks.InstallFile{File: nodetasks.File{ | |||
There was a problem hiding this comment.
A little awkward codewise, but I see the value!
(Would be great if these ended up as interfaces where the File task could implement both, but we have that conflict on Run... we could get rid of Run maybe, but ... not in this PR!)
|
/approve Thanks - would be nice if we didn't have to wrap File and Service, but it seems like we're moving forwards here! |
k8s-ci-robot
added
lgtm
|
I was actually wrapping in the other direction until I figured I was touching too much code. |
johngmyers
force-pushed
the
nodeup-install
branch
from
759d2c9 to
9b22483
Compare
k8s-ci-robot
removed
the
lgtm
johngmyers
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
/approve |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This clarifies that the phase that installs the kops-configuration unit needs a lot less state. It sets us up to move more state into
NodeupSubContextwithout having to create fake state in the install path.