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] Provide control over local override features #2404

Closed
fabriziopandini opened this issue Feb 22, 2020 · 13 comments · Fixed by #2679
Closed

[clusterctl] Provide control over local override features #2404

fabriziopandini opened this issue Feb 22, 2020 · 13 comments · Fixed by #2679
Assignees
Labels
area/clusterctl Issues or PRs related to clusterctl help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor.
Milestone

Comments

@fabriziopandini
Copy link
Member

User Story

As a developer, I would like to run clusterctl tests in isolation (without local overrides interfering on the test)

Detailed Description

Clusterctk supports local overrides and this feature proved super useful fo far.
However, local overrides can interfere with e2e test running on a developer machine.

Anything else you would like to add:

I see two options:

  • use a well known OS variable to force clusterctl to ignore local overrides
  • make the override folder configurable (or assume the override folder should be on the same location of the config file, which is already configurable)

The change should be implemented here, but it is required to make all the information required for activating/deactivating local oveerides

/area clusterctl
/kind feature
/help

@k8s-ci-robot
Copy link
Contributor

@fabriziopandini:
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:

User Story

As a developer, I would like to run clusterctl tests in isolation (without local overrides interfering on the test)

Detailed Description

Clusterctk supports local overrides and this feature proved super useful fo far.
However, local overrides can interfere with e2e test running on a developer machine.

Anything else you would like to add:

I see two options:

  • use a well known OS variable to force clusterctl to ignore local overrides
  • make the override folder configurable (or assume the override folder should be on the same location of the config file, which is already configurable)

The change should be implemented here, but it is required to make all the information required for activating/deactivating local oveerides

/area clusterctl
/kind feature
/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added area/clusterctl Issues or PRs related to clusterctl help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. labels Feb 22, 2020
@vincepri
Copy link
Member

/milestone v0.3.0

+1 on making the user folder configurable

@k8s-ci-robot k8s-ci-robot added this to the v0.3.0 milestone Feb 24, 2020
@wfernandes
Copy link
Contributor

/assign

@vincepri
Copy link
Member

vincepri commented Mar 2, 2020

/milestone v0.3.x

@k8s-ci-robot k8s-ci-robot modified the milestones: v0.3.0, v0.3.x Mar 2, 2020
@ncdc
Copy link
Contributor

ncdc commented Mar 2, 2020

@wfernandes are you still planning on working on this?

@wfernandes
Copy link
Contributor

@ncdc Yes. I was planning on working on this but I haven't yet started work on it. I've been getting sidetracked with other issues.

@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 Mar 2, 2020
@wfernandes
Copy link
Contributor

wfernandes commented Mar 3, 2020

Thoughts on what needs to be done

  • Make override folder configurable. See if using the config file path would be a viable solution.
  • Make changes to clusterctl e2e tests to use configurable overrides directory.
  • Update docs

@wfernandes
Copy link
Contributor

So I'm going to start implementation but before I do, I'd like to get some consensus.

Instead of passing in another flag for overrides or OS env variable, I think it would make sense to use the same directory that clusterctl.yaml is in.
That is, if we do clusterctl --config=/my/dir/path/clusterctl.yaml, we would look for the overrides in /my/dir/path/overrides.
I'm using the overrides directory still so as to not break the current UX.

/cc @fabriziopandini

@fabriziopandini
Copy link
Member Author

@wfernandes this was one on my suggested options, so I'm ok

make the override folder configurable (or assume the override folder should be on the same location of the config file, which is already configurable)

not sure if linking the override folder to the config file is ok for @vincepri who expressed +1 for “make the override folder configurable“

@wfernandes
Copy link
Contributor

So after digging more into the codebase and implementing a solution, a thought came to me.

Do we need the overrides directory if we ask the user to update the ~/.cluster-api/clusterctl.yaml with the appropriate filepath url?

That is, if the users have their local releases in a directory like ~/workspace/releases/infra-aws/v0.5.0/infra-comp.yaml, they can just specify that path in the clusterctl.yaml for the aws provider.

Currently, we are dissecting the above path mentioned as

basepath:~/workspace/releases
providerID: infra-aws
version: v0.5.0
componentsPath: infra-comp.yaml

The above happens when we try and get the cluster-template.yaml and the metadata.yaml as well.

From a developer user experience, the local-overrides.py script can continue to put the information in ~/.cluster-api/overrides and then we can ask the user to update the provider url in the clusterctl.yaml (or do that for the developer in the script). We could (maybe should) rename the files created by the local-overrides.py as infrastructure-components-dev.yaml.

It seems that the getLocalOverrides only happens when getting the components.yaml. I'm happy to further discuss in slack and/or zoom. I'll be picking this issue back up next week. 🙂

/cc @fabriziopandini @vincepri

@fabriziopandini
Copy link
Member Author

@wfernandes let's zoom/chat when you pickup the issue again next week

I'm not sure I fully understand your idea above, but changing the provider URL in the config file can't fix the issue that arises when running E2E tests and existing overrides leak-in.

getLocalOverrides only happens when getting the components.yaml

Get local overrides is called when reading from a repository the component yaml, metadata and also cluster-templates

@wfernandes
Copy link
Contributor

After discussing with @fabriziopandini we decided on adding a property in the clusterctl config file that determines the overrides folder path.
So in clusterctl.yaml you can specify

overridesFolder: /some/dir/path

We will still default to ~/.cluster-api/overrides

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clusterctl Issues or PRs related to clusterctl help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants