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.

Sign up for GitHub

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

Jump to bottom

Use clouds.yaml file and yq to merge with existing user's config #79

Closed
wants to merge 1 commit into from
Closed

Use clouds.yaml file and yq to merge with existing user's config #79

wants to merge 1 commit into from

Conversation

stbenjam
Copy link
Member

@stbenjam stbenjam commented Oct 7, 2019

"metal3" overlaps with the one in OpenShift, and they use different
IP's. Someone working on both projects potentially needs to talk to
Ironic in 3 places:

    - Minikube
    - OpenShift Bootstrap
    - OpenShift Cluster

@metal3-io-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: stbenjam

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 7, 2019
@russellb
Copy link
Member

russellb commented Oct 7, 2019

There's another PR that's adding an option to use kinder instead of minikube, so maybe we shouldn't embed minikube in the name.

Maybe we should just rename the one in OpenShift ...

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

Naming things is hard :-( The problem is dev-scripts has had a clouds.yaml in there for a while, although if anyone should own the plain "metal3" name, I would suppose it should be metal3-dev-env.

@russellb
Copy link
Member

russellb commented Oct 7, 2019

Yeah, that was my thought, metal3-dev-env should own that, and openshift dev-scripts should use an openshift- prefix for its names

@stbenjam
Use clouds.yaml file and yq to merge with existing user's config
df3e596 
This changes the approach to use a clouds.yaml file in the repo, and
uses yq to merge with a user's existing clouds.yaml. This has the
benefit of overriding the existing "metal3" config if it is ever
updated.

We are also renaming the clouds.yaml configurations in OpenShift's
dev-scripts, which previously used the metal3 name.
@hardys
Copy link
Member

hardys commented Oct 7, 2019

won't the IP be the same by default in the minikube and bootstrap VM case, e.g the names could overlap in that case?

@hardys
Copy link
Member

hardys commented Oct 7, 2019

Although in dev-scripts case we could just rename to bootstrap and cluster I guess

@metal3-io-bot metal3-io-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 7, 2019
@stbenjam stbenjam changed the title Rename metal3 clouds.yaml name Use clouds.yaml file and yq to merge with existing user's config Oct 7, 2019
@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019 •
edited
Loading

won't the IP be the same by default in the minikube and bootstrap VM case, e.g the names could overlap in that case?

They do for now, but what name would make sense? I think it makes sense for them to be separate, in case anything ever needs changing (e.g. if IP's become configurable).

I proposed openshift-metal3/dev-scripts#821, which adds an openshift prefix, and updated the approach here. It all uses yq and existing files should be handled nicer with their values being merged (although comments will be removed because yq roundtrips to JSON and back).

Open to a better naming scheme, openshift-metal3-bootstrap is a pain to type.

@stbenjam
Copy link
Member Author

stbenjam commented Oct 8, 2019

/close

@metal3-io-bot
Copy link
Collaborator

@stbenjam: Closed this PR.

In response to this:

/close

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.

@stbenjam
Copy link
Member Author

stbenjam commented Oct 8, 2019

#82 instead

@stbenjam stbenjam deleted the cloud-name branch October 8, 2019 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stbenjam @metal3-io-bot @russellb @hardys