Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clusterctl init --infrastructure aws --list-images returns an error when env variables are not defined #2516

Closed
detiber opened this issue Mar 3, 2020 · 15 comments · Fixed by #3059
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@detiber
Copy link
Member

detiber commented Mar 3, 2020

What steps did you take and what happened:

  • run clusterctl init --infrastructure aws --list-images without setting the AWS_B64ENCODED_CREDENTIALS env var
  • Get: Error: failed to get provider components for the "aws" provider: failed to perform variable substitution: value for variables [AWS_B64ENCODED_CREDENTIALS] is not set. Please set the value using os environment variables or the clusterctl config file

What did you expect to happen:

  • The command to run successfully without requiring the env variables to be set

Environment:

  • Cluster-api version: master
  • Minikube/KIND version: kind v0.6.1
  • Kubernetes version: (use kubectl version): v1.16.7
  • OS (e.g. from /etc/os-release): Fedora 31

/kind bug

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 3, 2020
@vincepri
Copy link
Member

vincepri commented Mar 4, 2020

/assign @fabriziopandini
/priority important-soon
/milestone v0.3.x

@k8s-ci-robot k8s-ci-robot added this to the v0.3.x milestone Mar 4, 2020
@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Mar 4, 2020
@fabriziopandini
Copy link
Member

@detiber @vincepri this is tricky because there is no guarantee that a YAML without env variables is a valid YAML.

e.g nothing prevents that the variable is the image repository or the image tag

In other words, from a clusterctl PoV only a fully compiled YAML can be input for further processing, like e.g. inspecting images.

@vincepri
Copy link
Member

Has this been fixed?

@kferrone
Copy link

kferrone commented May 1, 2020

I'm getting the same issue too: #2990

@fabriziopandini
Copy link
Member

@vincepri no this is not fixed.
clusterctl init requires all the variables to be in place, so it does when called with the --list-variables flag.

@wfernandes wdyt about making clusterctl init tolerate missing variable only when invoked with the --list-variables flag?

@fabriziopandini
Copy link
Member

@kferrone this is slightly different from #2990; the former is fixed by v03.5, this one not

@vincepri
Copy link
Member

vincepri commented May 4, 2020

/milestone v0.3.6

@k8s-ci-robot k8s-ci-robot modified the milestones: v0.3.x, v0.3.6 May 4, 2020
@wfernandes
Copy link
Contributor

Yeah..this is interesting.

In other words, from a clusterctl PoV only a fully compiled YAML can be input for further processing, like e.g. inspecting images.

I agree with @fabriziopandini that ideally, we should be inspecting the images once a yaml is fully processed. Because if provider components can also be templated then the images themselves could be variables that require processing.

wdyt about making clusterctl init tolerate missing variable only when invoked with the --list-variables flag?

@fabriziopandini Did you mean --list-images flag for this specific issue?

@vincepri
Copy link
Member

vincepri commented May 5, 2020

--list-images isn't really an option for init from what I can tell, does it makes sense to make it a separate command instead?

@wfernandes
Copy link
Contributor

I believe --list-images is a flag for clusterctl init.
--list-variables is a flag for clusterctl config cluster.

  # Lists the container images required for initializing the management cluster.
  #
  # Note: This command is a dry-run; it won't perform any action other than printing to screen.
  clusterctl init --infrastructure aws --list-images

does it makes sense to make it a separate command instead

However, I've come across this exact statement before.

// TODO: Move this to a sub-command or similar, it shouldn't really be a flag.
initCmd.Flags().BoolVar(&initOpts.listImages, "list-images", false,
"Lists the container images required for initializing the management cluster (without actually installing the providers)")

IMO we should deprecate this flag entirely and not substitute it with anything.
If a user wants to know what images they are using when trying to
clusterctl init --infrastructure aws, they should be using
clusterctl config provider --infrastrucuture aws.
This will give them the information they need without having to specify the config variable AWS_B64ENCODED_CREDENTIALS.

@fabriziopandini
Copy link
Member

IMO we should deprecate this flag entirely and not substitute it with anything.

The --list-images on clusterctl init is really helpful for the air-gapped scenario, because it gives you a list of all the required images in a single command.
If we drop this flag, the user will be required to call 4 times clusterctl config provider; additionally, the input of clusterctl config provider can't be easily piped to other commands, while the output of --list-images is designed for this.

Did you mean --list-images flag for this specific issue?

I mean that in case of clusterctl init called with --list-images, we can implicitly set skip-variables. In fact, we are already doing this for clusterctl config provider

@wfernandes
Copy link
Contributor

@fabriziopandini Thanks for the context about the need for --list-images. TIL. 🙂

I'm fine with adding the skip variables bool in the call for InitImages. Did we want to move it to a separate sub-command or can we do that as part of a separate issue?

@fabriziopandini
Copy link
Member

@wfernandes I'm in favor of keeping it simple and only changing the behavior of --list-images (without creating a new command).

But I defer this to @vincepri

@wfernandes
Copy link
Contributor

/assign

@wfernandes
Copy link
Contributor

/lifecycle active

@k8s-ci-robot k8s-ci-robot added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label May 12, 2020
@vincepri vincepri modified the milestones: v0.3.6, v0.3.x May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants