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

Make provider support more modular #795

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
5 changes: 3 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ COPY go.sum go.sum
RUN go mod download

# Copy the go source
COPY cmd/main.go cmd/main.go
COPY cmd/app.go cmd/app.go
COPY api/ api/
COPY internal/ internal/
COPY pkg/ pkg/

# Build
# the GOARCH has not a default value to allow the binary be built according to the host where the command
# was called. For example, if we call make docker-build in a local env which has the Apple Silicon M1 SO
# the docker BUILDPLATFORM arg will be linux/arm64 when for Apple x86 it will be linux/amd64. Therefore,
# by leaving it empty we can ensure that the container and binary shipped on it will have the same platform.
RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -ldflags="${LD_FLAGS}" -a -o manager cmd/main.go
RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -ldflags="${LD_FLAGS}" -a -o manager cmd/app.go

# Use distroless as minimal base image to package the manager binary
# Refer to https://github.com/GoogleContainerTools/distroless for more details
Expand Down
6 changes: 0 additions & 6 deletions api/v1alpha1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@ type (
)

const (
// Provider CAPA
ProviderAWSName = "cluster-api-provider-aws"
// Provider Azure
ProviderAzureName = "cluster-api-provider-azure"
// Provider vSphere
ProviderVSphereName = "cluster-api-provider-vsphere"
// Provider K0smotron
ProviderK0smotronName = "k0smotron"
// Provider Sveltos
Expand Down
14 changes: 4 additions & 10 deletions api/v1alpha1/management_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,23 +90,17 @@ type Provider struct {
Name string `json:"name"`
}

func (p Provider) String() string {
return p.Name
}

func (in *Component) HelmValues() (values map[string]any, err error) {
if in.Config != nil {
err = yaml.Unmarshal(in.Config.Raw, &values)
}
return values, err
}

func GetDefaultProviders() []Provider {
return []Provider{
{Name: ProviderK0smotronName},
{Name: ProviderAWSName},
{Name: ProviderAzureName},
{Name: ProviderVSphereName},
{Name: ProviderSveltosName},
}
}

// Templates returns a list of provider templates explicitly defined in the Management object
func (in *Management) Templates() []string {
templates := []string{}
Expand Down
29 changes: 29 additions & 0 deletions cmd/app.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2024
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package main

import (
"os"

"github.com/Mirantis/hmc/pkg/manager"
_ "github.com/Mirantis/hmc/pkg/providers/azure"
Copy link
Member

Choose a reason for hiding this comment

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

the aws provider is missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, due to how unit-tests are structured aws is enabled unconditionally.

Basically, unit-tests (mostly webhooks) use aws fixtures for testing, so there is no reason to have that provider optionally disable and will also break tests.

To make this possible some dummy provider is needed, or we can have aws and avoid questions why dummy provider exists

Copy link
Member

Choose a reason for hiding this comment

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

uh, I see, so basically in terms of runtime, the manager (binary) will have all of the providers (k0s, sveltos (included by default), azure's and vsphere's init functions are called here, and the aws comes from the tests themselves), correct? if so, i'd comment on it explicitly otherwise it's not very clear (to me personally) from first glance

Copy link
Contributor Author

@s3rj1k s3rj1k Dec 16, 2024

Choose a reason for hiding this comment

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

Not from tests, due to tests aws is registered in, which makes aws a Tier-1 provider.

I've gently removed azure unit-tests dependency, so it's only aws is like this, forever loved and supported provider.

This basically tells this story, not sure that whole unit-test story needs to be put in comment, if you think this will benefit us, please suggest what to put there.

Copy link
Member

Choose a reason for hiding this comment

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

yep, due to the import from the tests. that's clear. ok then, I think we can skip this one, I will leave this comment for a while if any of the rest reviewers would like to participate, otherwise, I'll resolve it

_ "github.com/Mirantis/hmc/pkg/providers/vsphere"
)

func main() {
if err := manager.Main(); err != nil {
os.Exit(1)
}
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ require (
github.com/fluxcd/pkg/apis/meta v1.8.0
github.com/fluxcd/pkg/runtime v0.50.1
github.com/fluxcd/source-controller/api v1.4.1
github.com/go-logr/logr v1.4.2
github.com/google/uuid v1.6.0
github.com/hashicorp/go-retryablehttp v0.7.7
github.com/onsi/ginkgo/v2 v2.22.1
Expand Down Expand Up @@ -84,7 +85,6 @@ require (
github.com/go-errors/errors v1.5.1 // indirect
github.com/go-gorp/gorp/v3 v3.1.0 // indirect
github.com/go-ldap/ldap/v3 v3.4.8 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-logr/zapr v1.3.0 // indirect
github.com/go-openapi/jsonpointer v0.21.0 // indirect
Expand Down
98 changes: 31 additions & 67 deletions internal/controller/managedcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ import (
"sigs.k8s.io/controller-runtime/pkg/predicate"

hmc "github.com/Mirantis/hmc/api/v1alpha1"
"github.com/Mirantis/hmc/internal/credspropagation"
"github.com/Mirantis/hmc/internal/helm"
"github.com/Mirantis/hmc/internal/sveltos"
"github.com/Mirantis/hmc/internal/telemetry"
"github.com/Mirantis/hmc/internal/utils/status"
"github.com/Mirantis/hmc/pkg/credspropagation"
providersloader "github.com/Mirantis/hmc/pkg/providers"
)

const (
Expand Down Expand Up @@ -575,35 +576,16 @@ func (r *ManagedClusterReconciler) releaseCluster(ctx context.Context, namespace
return err
}

var (
gvkAWSCluster = schema.GroupVersionKind{
Group: "infrastructure.cluster.x-k8s.io",
Version: "v1beta2",
Kind: "AWSCluster",
}

gvkAzureCluster = schema.GroupVersionKind{
Group: "infrastructure.cluster.x-k8s.io",
Version: "v1beta1",
Kind: "AzureCluster",
}

gvkMachine = schema.GroupVersionKind{
Group: "cluster.x-k8s.io",
Version: "v1beta1",
Kind: "Machine",
}
)

providerGVKs := map[string]schema.GroupVersionKind{
"aws": gvkAWSCluster,
"azure": gvkAzureCluster,
gvkMachine := schema.GroupVersionKind{
Group: "cluster.x-k8s.io",
Version: "v1beta1",
Kind: "Machine",
}

// Associate the provider with it's GVK
for _, provider := range providers {
gvk, ok := providerGVKs[provider]
if !ok {
gvk := providersloader.GetClusterGVK(provider)
if !gvk.Empty() {
continue
}

Expand Down Expand Up @@ -637,13 +619,12 @@ func (r *ManagedClusterReconciler) getInfraProvidersNames(ctx context.Context, t
return nil, err
}

const infraPrefix = "infrastructure-"
var (
ips = make([]string, 0, len(template.Status.Providers))
lprefix = len(infraPrefix)
lprefix = len(providersloader.InfraPrefix)
)
for _, v := range template.Status.Providers {
if idx := strings.Index(v, infraPrefix); idx > -1 {
if idx := strings.Index(v, providersloader.InfraPrefix); idx > -1 {
ips = append(ips, v[idx+lprefix:])
}
}
Expand Down Expand Up @@ -719,54 +700,37 @@ func (r *ManagedClusterReconciler) reconcileCredentialPropagation(ctx context.Co
}

for _, provider := range providers {
switch provider {
case "aws":
l.Info("Skipping creds propagation for AWS")
case "azure":
l.Info("Azure creds propagation start")
if err := credspropagation.PropagateAzureSecrets(ctx, propnCfg); err != nil {
errMsg := fmt.Sprintf("failed to create Azure CCM credentials: %s", err)
apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{
Type: hmc.CredentialsPropagatedCondition,
Status: metav1.ConditionFalse,
Reason: hmc.FailedReason,
Message: errMsg,
})

return errors.New(errMsg)
}
titleName := providersloader.GetProviderTitleName(provider)

f, ok := providersloader.CredentialPropagationFunc(provider)
if !ok || titleName == "" {
apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{
Type: hmc.CredentialsPropagatedCondition,
Status: metav1.ConditionTrue,
Reason: hmc.SucceededReason,
Message: "Azure CCM credentials created",
Status: metav1.ConditionFalse,
Reason: hmc.FailedReason,
Message: "unsupported infrastructure provider " + provider,
})
case "vsphere":
l.Info("vSphere creds propagation start")
if err := credspropagation.PropagateVSphereSecrets(ctx, propnCfg); err != nil {
errMsg := fmt.Sprintf("failed to create vSphere CCM credentials: %s", err)
apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{
Type: hmc.CredentialsPropagatedCondition,
Status: metav1.ConditionFalse,
Reason: hmc.FailedReason,
Message: errMsg,
})
return errors.New(errMsg)
}

continue
}

enabled, err := f(ctx, propnCfg, l)
if err != nil {
errMsg := fmt.Sprintf("failed to create %s CCM credentials: %s", titleName, err)
apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{
Type: hmc.CredentialsPropagatedCondition,
Status: metav1.ConditionTrue,
Reason: hmc.SucceededReason,
Message: "vSphere CCM credentials created",
Status: metav1.ConditionFalse,
Reason: hmc.FailedReason,
Message: errMsg,
})
default:

return errors.New(errMsg)
} else if enabled {
apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{
Type: hmc.CredentialsPropagatedCondition,
Status: metav1.ConditionFalse,
Reason: hmc.FailedReason,
Message: "unsupported infrastructure provider " + provider,
Status: metav1.ConditionTrue,
Reason: hmc.SucceededReason,
Message: titleName + " CCM credentials created",
})
}
}
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/managedcluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

hmc "github.com/Mirantis/hmc/api/v1alpha1"
providersloader "github.com/Mirantis/hmc/pkg/providers"
)

var _ = Describe("ManagedCluster Controller", func() {
Expand Down Expand Up @@ -97,7 +98,7 @@ var _ = Describe("ManagedCluster Controller", func() {
Raw: []byte(`{"foo":"bar"}`),
},
},
Providers: hmc.Providers{"infrastructure-aws"},
Providers: hmc.Providers{providersloader.InfraPrefix + "aws"},
}
Expect(k8sClient.Status().Update(ctx, template)).To(Succeed())
}
Expand Down Expand Up @@ -144,7 +145,7 @@ var _ = Describe("ManagedCluster Controller", func() {
}
Expect(k8sClient.Create(ctx, management)).To(Succeed())
management.Status = hmc.ManagementStatus{
AvailableProviders: hmc.Providers{"infrastructure-aws"},
AvailableProviders: hmc.Providers{providersloader.InfraPrefix + "aws"},
}
Expect(k8sClient.Status().Update(ctx, management)).To(Succeed())
}
Expand Down
3 changes: 2 additions & 1 deletion internal/controller/release_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"github.com/Mirantis/hmc/internal/build"
"github.com/Mirantis/hmc/internal/helm"
"github.com/Mirantis/hmc/internal/utils"
"github.com/Mirantis/hmc/pkg/providers"
)

// ReleaseReconciler reconciles a Template object
Expand Down Expand Up @@ -194,7 +195,7 @@ func (r *ReleaseReconciler) ensureManagement(ctx context.Context) error {
if err != nil {
return err
}
mgmtObj.Spec.Providers = hmc.GetDefaultProviders()
mgmtObj.Spec.Providers = providers.List()

getter := helm.NewMemoryRESTClientGetter(r.Config, r.RESTMapper())
actionConfig := new(action.Configuration)
Expand Down
29 changes: 11 additions & 18 deletions internal/webhook/managedcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

hmcv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1"
providersloader "github.com/Mirantis/hmc/pkg/providers"
)

type ManagedClusterValidator struct {
Expand Down Expand Up @@ -239,7 +240,7 @@ func (v *ManagedClusterValidator) validateCredential(ctx context.Context, manage

hasInfra := false
for _, v := range template.Status.Providers {
if strings.HasPrefix(v, "infrastructure-") {
if strings.HasPrefix(v, providersloader.InfraPrefix) {
hasInfra = true
break
}
Expand Down Expand Up @@ -269,25 +270,17 @@ func isCredMatchTemplate(cred *hmcv1alpha1.Credential, template *hmcv1alpha1.Clu
}

for _, provider := range template.Status.Providers {
switch provider {
case "infrastructure-aws":
if idtyKind != "AWSClusterStaticIdentity" &&
idtyKind != "AWSClusterRoleIdentity" &&
idtyKind != "AWSClusterControllerIdentity" {
return errMsg(provider)
}
case "infrastructure-azure":
if idtyKind != "AzureClusterIdentity" {
return errMsg(provider)
}
case "infrastructure-vsphere":
if idtyKind != "VSphereClusterIdentity" {
return errMsg(provider)
}
default:
if strings.HasPrefix(provider, "infrastructure-") {
idtys, found := providersloader.GetClusterIdentityKind(provider)
if !found {
if strings.HasPrefix(provider, providersloader.InfraPrefix) {
return fmt.Errorf("unsupported infrastructure provider %s", provider)
}

continue
}

if !slices.Contains(idtys, idtyKind) {
return errMsg(provider)
}
}

Expand Down
Loading
Loading