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

Add OpenStack provider support with configuration and templates #695

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add OpenStack provider support with configuration and templates #695

wants to merge 1 commit into from

Conversation

bnallapeta
Copy link
Member

@bnallapeta bnallapeta commented Nov 28, 2024 •
edited
Loading

Resolves #594.

OpenStack Provider Integration:

  • Introduces OpenStack as a supported cloud provider in HMC.
  • Adds configurations and provider templates specific to OpenStack environments.

@bnallapeta bnallapeta mentioned this pull request Nov 28, 2024
Open
@bnallapeta bnallapeta force-pushed the openstack-add-provider branch 2 times, most recently from ff0b2d1 to 5b2932d Compare November 28, 2024 12:57
a13x5
a13x5 requested changes Nov 28, 2024
View reviewed changes
Copy link
Contributor

@a13x5 a13x5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird imports should be removed (or moved to another PR).
Minor clarifications are needed.

cmd/main.go Outdated Show resolved Hide resolved
cmd/main.go Outdated Show resolved Hide resolved
templates/provider/cluster-api-provider-openstack/values.yaml
@@ -0,0 +1,6 @@
configSecret:
create: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to create an empty config?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are using config in secret.yaml. If this is not defined, helm template will throw an error. It also lets the users know what fields are available in values.yaml

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean you have empty .Values.config by default and thus secret will be created with and empty data (which is weird). So if controller will work fine without the empty secret I suggest setting configSecret.create: false and skip the creation of the secret

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I did test it with the current way which is an empty config. But I will test this again to make sure it works and then make it false.

a13x5
a13x5 requested changes Nov 28, 2024
View reviewed changes
templates/provider/cluster-api-provider-openstack/values.yaml
@@ -0,0 +1,6 @@
configSecret:
create: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean you have empty .Values.config by default and thus secret will be created with and empty data (which is weird). So if controller will work fine without the empty secret I suggest setting configSecret.create: false and skip the creation of the secret

go.mod Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If imports aren't needed, dependencies in the go.mod/go.sum aren't needed as well. Remove.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still see them in the diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a13x5 a13x5 a13x5 requested changes

@Kshatrix Kshatrix Awaiting requested review from Kshatrix Kshatrix is a code owner

@eromanova eromanova Awaiting requested review from eromanova eromanova is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
Project 2A
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OpenStack provider template for HMC
3 participants
@bnallapeta @eromanova @a13x5