search

kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.5k stars 1.3k forks source link

clusterctl --config=<config> should support URL #3099

Closed deitch closed 4 years ago

deitch commented 4 years ago

User Story

As a user I would like to run clusterctl --config=<url> so that I can reference providers out there in a single step, without downloading them.

Detailed Description

If I do clusterctl --config=https://github.com/some/path/to/clusterctl.yaml, it fails without a clear understanding.

Separately, if it does not support a URL, it should explain that it cannot find that path (since it is not a path), but more fundamentally, it should support a URL.

This makes it much easier to use external providers, as opposed to requiring a curl or other download to local file followed by a clusterctl --config.

/kind feature

deitch commented 4 years ago

Also, it would be great if it had the same "resolve to asset" logic that we use in the built-in repositories, where we can do URL=https://github.com/kubernetes-sigs/cluster-api-provider-aws/releases/latest/ and FILE=infrastructure-components.yaml and it knows how to resolve to the release pointed to by the URL, and then find the asset. Perhaps even --config=<URL>, where <URL>=url-to-release, and it knows to look for clusterctl.yaml in there.

vincepri commented 4 years ago

/assign @wfernandes @fabriziopandini

fabriziopandini commented 4 years ago

I'm +1 in getting --config to support https/http if this can help users in getting started.

WRT to the idea of supporting latest in --config I'm a little bit more cautious because I don't want to over-engineer get config. Also, AFAIK https://github.com/kubernetes-sigs/cluster-api/releases/latest automatically resolve to latest, so this should be covered without additional logic

deitch commented 4 years ago

because I don't want to over-engineer get config

Neither do I. If we can get it directly from a URL asset, then all good.

wfernandes commented 4 years ago

@deitch Thanks for this issue however I don't think I understand the use case. clusterctl.yaml is the configuration file for clusterctl itself and is not specific to any provider.

So if a provider provides clusterctl.yaml as part of its releases, will it be used to provide default values for provider's cluster templates?

What about the other "global" properties defined in the clusterctl.yaml? For example, CLUSTERCTL_LOG_LEVEL, overridesFolder, cert-manager-timeout, or the providers section.

I think as @fabriziopandini pointed out, this would be fine for users getting started but as users start using multiple providers, this could get interesting.

deitch commented 4 years ago

Thanks for the detailed comments @wfernandes

I almost wrote “I don’t know”, but then realized, “yes, I do”.

The issue you are raising is, I believe, that the config file serves multiple purposes, but does not provide a way to separate those purposes or to compose them. Those purposes are:

If I want to keep my normal config but also add in one more infra and one more bootstrap provider, on a one off basis, each of which is in a different location, I’m stuck. I can edit my own file, which is messy and is not conducive to easy-to-start directions.

All of the above is independent of the use case that maybe I just do want to use a shared config file from a url.

Addressing the two cases. For the latter - I really do just want to use a shared config from a url - the answer is to have --config support full https URLs

For the former - composing and different functions - I see two possibilities.

One possibility is that I can have multiple config options, and they stack or layer, rather than replace. I’m ok with that, although it can be hard to determine when I mean to replace my config, and when I meant to add to it. And it would have to support urls.

The other is to have a separate option that lets me specify a url. So I can do 

clusterctl init -infrastructure=myprovider:<url>

There are other ways to do that syntax, but the general idea, that I can augment providers by specifying their config definition, is the basic idea.

vincepri commented 4 years ago

/milestone Next

Setting Next until we have agreed on a clear design / path forward

fabriziopandini commented 4 years ago

as said before supporting https/http source for config is not a bad idea, especially if it can avoid config editing during quick-start afterwards, the user can shift to local config, or a team can agree on managing a shared config on a remote location.

@deitch FYI, if your concert is to provider an easy start for user, you can also send a PR for embedding your provider config in clusterctl here

deitch commented 4 years ago

FYI, if your concert is to provider an easy start for user, you can also send a PR for embedding your provider config in clusterctl

@fabriziopandini LOL! I literally have a window open typing git commit on a branch to do precisely that! Your timing is perfect. 😀

fabriziopandini commented 4 years ago

Great! then you might be interested in providing your settings here as well

deitch commented 4 years ago

I did that too. I am building locally and testing before pushing out the commit and opening the PR.

vincepri commented 4 years ago

/priority backlog /help

k8s-ci-robot commented 4 years ago

@vincepri: This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/kubernetes-sigs/cluster-api/issues/3099): >/priority backlog >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
cpanato commented 4 years ago

@fabriziopandini @vincepri what is the decision here, the flag --config accept https/http config? was not clear to me.

fabriziopandini commented 4 years ago

I'm +1 to support urls in --config

wfernandes commented 4 years ago

I'm fine with adding URL support for --config.

One thing to take into account regarding UX. Currently, other options for pulling from a URL are a bit specific. For example, in clusterctl config cluster --from <url>, here the URL must be from github.com and the local file system and NOT any random url.

So as per the original suggestion above from @deitch:

Also, it would be great if it had the same "resolve to asset" logic that we use in the built-in repositories, where we can do URL=https://github.com/kubernetes-sigs/cluster-api-provider-aws/releases/latest/ and FILE=infrastructure-components.yaml and it knows how to resolve to the release pointed to by the URL, and then find the asset. Perhaps even --config=, where =url-to-release, and it knows to look for clusterctl.yaml in there.

I think for now we could support the above use case quite easily.

cpanato commented 4 years ago

@wfernandes I did a bit different, but I'm willing to change if we all agree in the suggestion above, I did not implement in this way because the user might have a different name for the config file.

fabriziopandini commented 4 years ago

/milestone v0.3.10 given that the PR is getting ready