Standardize dependency checking for kne deploy #150
Conversation
Pull Request Test Coverage Report for Build 2360529480
💛 - Coveralls |
deploy/deploy.go
Outdated
| func checkDependencies() error { | ||
| bins := []string{"docker", "kubectl"} | ||
| var errs errlist.List | ||
| for _, bin := range bins { |
There was a problem hiding this comment.
for _, bin := []string{"docker", "kubectl"}
There was a problem hiding this comment.
done, will push a commit soon
deploy/deploy.go
Outdated
| return nil | ||
| } | ||
| } | ||
| if _, err := execLookPath("kind"); err != nil { |
There was a problem hiding this comment.
maybe we should codify for each spec a "depends" on func like you did above but attach it to the struct?
There was a problem hiding this comment.
yeah I agree, we can have a depends for the spec, additionally options such as googleArtifactRegistry access requires an additional dep (gcloud) which we could check in the depends func based on the optional fields provided
| return errors.Wrap(err, "failed to create cluster using kind client") | ||
| if k.Recycle { | ||
| log.Infof("Attempting to recycle existing cluster %q...", k.Name) | ||
| if err := execer.Exec("kubectl", "cluster-info", "--context", fmt.Sprintf("kind-%s", k.Name)); err == nil { |
There was a problem hiding this comment.
should you have a dep check on kubectl? - like is there a guarantee that this order is always maintained?
There was a problem hiding this comment.
I see two ways of doing it:
-
having each "Spec" list out the dependencies and call checkDependencies for each one before any method
-
having docker and kubectl be dependencies of any deployment and have the spec dependencies only include the remainder of the dependencies (this is the standard that this PR implements)
I can see potentially a 3rd design that steals from (1) and (2) where docker and kubectl are checked for a deployment regardless of the specs and spec dependencies are checked before ever method call, not just deploy.
I think the 3rd option is the best considering these specs should not be called outside of Deployment.Deploy() and are only exported due to yaml marshaling
Before the dependency checking was not uniform, some methods checked for required binaries and others didn't.
Now "docker" and "kubectl" are considered dependencies to make any deployment
"kind" is a dependency to make deployments with kind cluster
"gcloud" is only a dependency when using kind linked with GAR
Other changes: