Skip to content

🌱 clusterctl: add nutanix ipam & runtime extensions providers #11135

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

Merged
merged 3 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions cmd/clusterctl/client/config/providers_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,19 @@ const (
// IPAM providers.
const (
InClusterIPAMProviderName = "in-cluster"
NutanixIPAMProviderName = "nutanix"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
NutanixIPAMProviderName = "nutanix"
NutanixIPAMProviderName = "nutanix-cloud-native-nutanix"

To follow the conventions at https://main.cluster-api.sigs.k8s.io/clusterctl/provider-contract.html#adding-a-provider-to-clusterctl

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 already have an infra provider simply called nutanix at https://github.com/kubernetes-sigs/cluster-api/blob/main/cmd/clusterctl/client/config/providers_client.go#L55 and it would be great if we could use the same name for these other provider types for consistency 🙏

Copy link
Member

Choose a reason for hiding this comment

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

Good point. Maybe it's fine if a GitHub org already "claimed" the name via another provider

Copy link
Member

Choose a reason for hiding this comment

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

Would make sense. Fine for me if we have an agreement :-)

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I think we can allow this because https://github.com/nutanix is owned by the same org that owns https://github.com/nutanix-cloud-native.

Do you mind updating https://cluster-api.sigs.k8s.io/clusterctl/provider-contract.html#providers-github-org-prefix with something like

In case the entity proposing the new providers owns different github organizations, it is also possible to Provider's GitHub org prefix (in this case it is up to this entity to avoid or address conflicts).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure we need to update any docs. Can't we just agree as we (Nutanix) already claimed the nutanix provider name for infra provider as mentioned above?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added some docs as requested. Please review when you get a chance 🙏

)

// Add-on providers.
const (
HelmAddonProviderName = "helm"
)

// Runtime extensions providers.
const (
NutanixRuntimeExtensionsProviderName = "nutanix"
)

// Other.
const (
// ProvidersConfigKey is a constant for finding provider configurations with the ProvidersClient.
Expand Down Expand Up @@ -422,13 +428,25 @@ func (p *providersClient) defaults() []Provider {
url: "https://github.com/kubernetes-sigs/cluster-api-ipam-provider-in-cluster/releases/latest/ipam-components.yaml",
providerType: clusterctlv1.IPAMProviderType,
},
&provider{
name: NutanixIPAMProviderName,
url: "https://github.com/nutanix-cloud-native/cluster-api-ipam-provider-nutanix/releases/latest/ipam-components.yaml",
providerType: clusterctlv1.IPAMProviderType,
},

// Add-on providers
&provider{
name: HelmAddonProviderName,
url: "https://github.com/kubernetes-sigs/cluster-api-addon-provider-helm/releases/latest/addon-components.yaml",
providerType: clusterctlv1.AddonProviderType,
},

// Runtime extensions providers
&provider{
name: NutanixRuntimeExtensionsProviderName,
url: "https://github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/releases/latest/runtime-extensions-components.yaml",
providerType: clusterctlv1.RuntimeExtensionProviderType,
},
}

return defaults
Expand Down
4 changes: 4 additions & 0 deletions cmd/clusterctl/client/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ func Test_clusterctlClient_GetProvidersConfig(t *testing.T) {
config.VSphereProviderName,
config.VultrProviderName,
config.InClusterIPAMProviderName,
config.NutanixIPAMProviderName,
config.NutanixRuntimeExtensionsProviderName,
config.HelmAddonProviderName,
},
wantErr: false,
Expand Down Expand Up @@ -174,6 +176,8 @@ func Test_clusterctlClient_GetProvidersConfig(t *testing.T) {
config.VSphereProviderName,
config.VultrProviderName,
config.InClusterIPAMProviderName,
config.NutanixIPAMProviderName,
config.NutanixRuntimeExtensionsProviderName,
config.HelmAddonProviderName,
},
wantErr: false,
Expand Down
Loading