Skip to content

Commit

Permalink
Fix providers status reporting in management object
Browse files Browse the repository at this point in the history
Closes #581
  • Loading branch information
eromanova authored and Kshatrix committed Nov 20, 2024
1 parent aba50e5 commit 10e2719
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 23 deletions.
13 changes: 12 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,10 @@ SVELTOS_VERSION ?= v$(shell $(YQ) -r '.appVersion' $(PROVIDER_TEMPLATES_DIR)/pro
SVELTOS_NAME ?= sveltos
SVELTOS_CRD ?= $(EXTERNAL_CRD_DIR)/$(SVELTOS_NAME)-$(SVELTOS_VERSION).yaml

CAPI_OPERATOR_VERSION ?= v$(shell $(YQ) -r '.dependencies.[] | select(.name == "cluster-api-operator") | .version' $(PROVIDER_TEMPLATES_DIR)/hmc/Chart.yaml)
CAPI_OPERATOR_CRD_PREFIX ?= "operator.cluster.x-k8s.io_"
CAPI_OPERATOR_CRDS ?= capi-operator-crds

## Tool Binaries
KUBECTL ?= kubectl
CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen-$(CONTROLLER_TOOLS_VERSION)
Expand Down Expand Up @@ -479,8 +483,15 @@ $(SVELTOS_CRD): | yq $(EXTERNAL_CRD_DIR)
rm -f $(EXTERNAL_CRD_DIR)/$(SVELTOS_NAME)*
curl -s --fail https://raw.githubusercontent.com/projectsveltos/sveltos/$(SVELTOS_VERSION)/manifest/crds/sveltos_crds.yaml > $(SVELTOS_CRD)

$(CAPI_OPERATOR_CRDS): | $(YQ) $(EXTERNAL_CRD_DIR)
rm -f $(EXTERNAL_CRD_DIR)/$(CAPI_OPERATOR_CRD_PREFIX)*
@$(foreach name, \
addonproviders bootstrapproviders controlplaneproviders coreproviders infrastructureproviders ipamproviders runtimeextensionproviders, \
curl -s --fail https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-operator/$(CAPI_OPERATOR_VERSION)/config/crd/bases/$(CAPI_OPERATOR_CRD_PREFIX)${name}.yaml \
> $(EXTERNAL_CRD_DIR)/$(CAPI_OPERATOR_CRD_PREFIX)${name}-$(CAPI_OPERATOR_VERSION).yaml;)

.PHONY: external-crd
external-crd: $(FLUX_HELM_CRD) $(FLUX_SOURCE_CHART_CRD) $(FLUX_SOURCE_REPO_CRD) $(SVELTOS_CRD)
external-crd: $(FLUX_HELM_CRD) $(FLUX_SOURCE_CHART_CRD) $(FLUX_SOURCE_REPO_CRD) $(SVELTOS_CRD) $(CAPI_OPERATOR_CRDS)

.PHONY: kind
kind: $(KIND) ## Download kind locally if necessary.
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ require (
k8s.io/client-go v0.31.2
k8s.io/utils v0.0.0-20241104163129-6fe5fd82f078
sigs.k8s.io/cluster-api v1.8.5
sigs.k8s.io/cluster-api-operator v0.14.0
sigs.k8s.io/cluster-api-provider-azure v1.17.1
sigs.k8s.io/cluster-api-provider-vsphere v1.11.3
sigs.k8s.io/controller-runtime v0.19.1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,8 @@ sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 h1:2770sDpzrjjsA
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw=
sigs.k8s.io/cluster-api v1.8.5 h1:lNA2fPN4fkXEs+oOQlnwxT/4VwRFBpv5kkSoJG8nqBA=
sigs.k8s.io/cluster-api v1.8.5/go.mod h1:pXv5LqLxuIbhGIXykyNKiJh+KrLweSBajVHHitPLyoY=
sigs.k8s.io/cluster-api-operator v0.14.0 h1:0QgO6+XGrNNJnNHKBwvQD5v6w+EaH3Z0RL1nL3wpjA4=
sigs.k8s.io/cluster-api-operator v0.14.0/go.mod h1:euShpVN6HyxXas28HkrYxhCPVDW1UV6ljbRBAeCxp8Y=
sigs.k8s.io/cluster-api-provider-azure v1.17.1 h1:f1sTGfv6hAN9WrxeawE4pQ2nRhEKb7AJjH6MhU/wAzg=
sigs.k8s.io/cluster-api-provider-azure v1.17.1/go.mod h1:16VtsvIpK8qtNHplG2ZHZ74/JKTzOUQIAWWutjnpvEc=
sigs.k8s.io/cluster-api-provider-vsphere v1.11.3 h1:ONrHsZgiR3L/W4572mn9xvdVnqE0fNogIJI5TeaQI0I=
Expand Down
74 changes: 54 additions & 20 deletions internal/controller/management_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import (
"strings"

fluxv2 "github.com/fluxcd/helm-controller/api/v2"
"github.com/fluxcd/pkg/apis/meta"
fluxmeta "github.com/fluxcd/pkg/apis/meta"
fluxconditions "github.com/fluxcd/pkg/runtime/conditions"
sourcev1 "github.com/fluxcd/source-controller/api/v1"
"helm.sh/helm/v3/pkg/chartutil"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
Expand All @@ -32,6 +33,7 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -118,6 +120,8 @@ func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Manag
components: make(map[string]hmc.ComponentStatus),
compatibilityContracts: make(map[string]hmc.CompatibilityContracts),
}

requeue bool
)
for _, component := range components {
l.V(1).Info("reconciling components", "component", component)
Expand Down Expand Up @@ -152,12 +156,11 @@ func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Manag
continue
}

if component.Template != hmc.CoreHMCName {
if err := r.checkProviderStatus(ctx, component.Template); err != nil {
updateComponentsStatus(statusAccumulator, component, nil, fmt.Sprintf("Failed to check provider status: %s", err))
errs = errors.Join(errs, err)
continue
}
if err := r.checkProviderStatus(ctx, component); err != nil {
l.Info("Provider is not yet ready", "template", component.Template, "err", err)
requeue = true
updateComponentsStatus(statusAccumulator, component, nil, err.Error())
continue
}

updateComponentsStatus(statusAccumulator, component, template, "")
Expand All @@ -177,6 +180,9 @@ func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Manag
l.Error(errs, "Multiple errors during Management reconciliation")
return ctrl.Result{}, errs
}
if requeue {
return ctrl.Result{RequeueAfter: DefaultRequeueInterval}, nil
}

return ctrl.Result{}, nil
}
Expand Down Expand Up @@ -256,9 +262,31 @@ func (r *ManagementReconciler) ensureTemplateManagement(ctx context.Context, mgm
// checkProviderStatus checks the status of a provider associated with a given
// ProviderTemplate name. Since there's no way to determine resource Kind from
// the given template iterate over all possible provider types.
func (r *ManagementReconciler) checkProviderStatus(ctx context.Context, providerTemplateName string) error {
var errs error
func (r *ManagementReconciler) checkProviderStatus(ctx context.Context, component component) error {
helmReleaseName := component.helmReleaseName
hr := &fluxv2.HelmRelease{}
err := r.Get(ctx, types.NamespacedName{Namespace: r.SystemNamespace, Name: helmReleaseName}, hr)
if err != nil {
return fmt.Errorf("failed to check provider status: %w", err)
}

hrReadyCondition := fluxconditions.Get(hr, fluxmeta.ReadyCondition)
if hrReadyCondition == nil || hrReadyCondition.ObservedGeneration != hr.Generation {
return fmt.Errorf("HelmRelease %s/%s Ready condition is not updated yet", r.SystemNamespace, helmReleaseName)
}
if !fluxconditions.IsReady(hr) {
return fmt.Errorf("HelmRelease %s/%s is not yet ready: %s", r.SystemNamespace, helmReleaseName, hrReadyCondition.Message)
}

if hr.Status.History.Latest() == nil {
return fmt.Errorf("HelmRelease %s/%s has empty deployment history in the status", r.SystemNamespace, helmReleaseName)
}

if !component.isCAPIProvider {
return nil
}
var errs error
var providerFound bool
for _, resourceType := range []string{
"coreproviders",
"infrastructureproviders",
Expand All @@ -272,29 +300,33 @@ func (r *ManagementReconciler) checkProviderStatus(ctx context.Context, provider
}

resourceConditions, err := status.GetResourceConditions(ctx, r.SystemNamespace, r.DynamicClient, gvr,
labels.SelectorFromSet(map[string]string{hmc.FluxHelmChartNameKey: providerTemplateName}).String(),
labels.SelectorFromSet(map[string]string{hmc.FluxHelmChartNameKey: hr.Status.History.Latest().Name}).String(),
)
if err != nil {
if errors.As(err, &status.ResourceNotFoundError{}) {
// Check the next resource type.
continue
}

return fmt.Errorf("failed to get status for template: %s: %w", providerTemplateName, err)
return fmt.Errorf("failed to get conditions from %s: %w", gvr.Resource, err)
}

var falseConditionTypes []string
providerFound = true

var falseConditionMessages []string
for _, condition := range resourceConditions.Conditions {
if condition.Status != metav1.ConditionTrue {
falseConditionTypes = append(falseConditionTypes, condition.Type)
falseConditionMessages = append(falseConditionMessages, condition.Message)
}
}

if len(falseConditionTypes) > 0 {
errs = errors.Join(errs, fmt.Errorf("%s: %s is not yet ready, has false conditions: %s",
resourceConditions.Name, resourceConditions.Kind, strings.Join(falseConditionTypes, ", ")))
if len(falseConditionMessages) > 0 {
errs = errors.Join(errs, fmt.Errorf("%s is not yet ready: %s",
resourceConditions.Kind, strings.Join(falseConditionMessages, ", ")))
}
}
if !providerFound {
return errors.New("waiting for Cluster API Provider objects to be created")
}

return errs
}
Expand Down Expand Up @@ -373,8 +405,9 @@ type component struct {
helmReleaseName string
targetNamespace string
// helm release dependencies
dependsOn []meta.NamespacedObjectReference
dependsOn []fluxmeta.NamespacedObjectReference
createNamespace bool
isCAPIProvider bool
}

func applyHMCDefaults(config *apiextensionsv1.JSON) (*apiextensionsv1.JSON, error) {
Expand Down Expand Up @@ -427,7 +460,7 @@ func getWrappedComponents(ctx context.Context, cl client.Client, mgmt *hmc.Manag

capiComp := component{
Component: mgmt.Spec.Core.CAPI, helmReleaseName: hmc.CoreCAPIName,
dependsOn: []meta.NamespacedObjectReference{{Name: hmc.CoreHMCName}},
dependsOn: []fluxmeta.NamespacedObjectReference{{Name: hmc.CoreHMCName}}, isCAPIProvider: true,
}
if capiComp.Template == "" {
capiComp.Template = release.Spec.CAPI.Template
Expand All @@ -439,7 +472,7 @@ func getWrappedComponents(ctx context.Context, cl client.Client, mgmt *hmc.Manag
for _, p := range mgmt.Spec.Providers {
c := component{
Component: p.Component, helmReleaseName: p.Name,
dependsOn: []meta.NamespacedObjectReference{{Name: hmc.CoreCAPIName}},
dependsOn: []fluxmeta.NamespacedObjectReference{{Name: hmc.CoreCAPIName}}, isCAPIProvider: true,
}
// Try to find corresponding provider in the Release object
if c.Template == "" {
Expand All @@ -449,6 +482,7 @@ func getWrappedComponents(ctx context.Context, cl client.Client, mgmt *hmc.Manag
if p.Name == hmc.ProviderSveltosName {
c.targetNamespace = sveltosTargetNamespace
c.createNamespace = true
c.isCAPIProvider = false
}

components = append(components, c)
Expand Down
98 changes: 96 additions & 2 deletions internal/controller/management_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ import (
"time"

helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2"
fluxmeta "github.com/fluxcd/pkg/apis/meta"
fluxconditions "github.com/fluxcd/pkg/runtime/conditions"
sourcev1 "github.com/fluxcd/source-controller/api/v1"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
capioperator "sigs.k8s.io/cluster-api-operator/api/v1alpha2"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

Expand Down Expand Up @@ -102,6 +106,20 @@ var _ = Describe("Management Controller", func() {
interval = time.Millisecond * 250
)

coreComponents := map[string]struct {
templateName string
helmReleaseName string
}{
"hmc": {
templateName: "test-release-hmc",
helmReleaseName: "hmc",
},
"capi": {
templateName: "test-release-capi",
helmReleaseName: "capi",
},
}

// NOTE: other tests for some reason are manipulating with the NS globally and interfer with each other,
// so try to avoid depending on their implementation ignoring its removal
By("Creating the hmc-system namespace")
Expand All @@ -120,8 +138,8 @@ var _ = Describe("Management Controller", func() {
},
Spec: hmcmirantiscomv1alpha1.ReleaseSpec{
Version: "test-version",
HMC: hmcmirantiscomv1alpha1.CoreProviderTemplate{Template: "test-release-hmc"},
CAPI: hmcmirantiscomv1alpha1.CoreProviderTemplate{Template: "test-release-capi"},
HMC: hmcmirantiscomv1alpha1.CoreProviderTemplate{Template: coreComponents["hmc"].templateName},
CAPI: hmcmirantiscomv1alpha1.CoreProviderTemplate{Template: coreComponents["capi"].templateName},
},
}
Expect(k8sClient.Create(ctx, release)).To(Succeed())
Expand Down Expand Up @@ -242,7 +260,79 @@ var _ = Describe("Management Controller", func() {
By("Checking the Management object does not have the removed component in its spec")
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(mgmt), mgmt)).To(Succeed())
Expect(mgmt.Status.AvailableProviders).To(BeEmpty())

By("Checking the Management components status is populated")

Expect(mgmt.Status.Components).To(HaveLen(2)) // required: capi, hmc
Expect(mgmt.Status.Components).To(BeEquivalentTo(map[string]hmcmirantiscomv1alpha1.ComponentStatus{
hmcmirantiscomv1alpha1.CoreHMCName: {
Success: false,
Template: providerTemplateRequiredComponent,
Error: fmt.Sprintf("HelmRelease %s/%s Ready condition is not updated yet", helmReleaseNamespace, coreComponents["hmc"].helmReleaseName),
},
hmcmirantiscomv1alpha1.CoreCAPIName: {
Success: false,
Template: providerTemplateRequiredComponent,
Error: fmt.Sprintf("HelmRelease %s/%s Ready condition is not updated yet", helmReleaseNamespace, coreComponents["capi"].helmReleaseName),
},
}))

By("Update core HelmReleases with Ready condition")

for _, coreComponent := range coreComponents {
helmRelease := &helmcontrollerv2.HelmRelease{}
err := k8sClient.Get(ctx, types.NamespacedName{
Namespace: helmReleaseNamespace,
Name: coreComponent.helmReleaseName,
}, helmRelease)
Expect(err).NotTo(HaveOccurred())

fluxconditions.Set(helmRelease, &metav1.Condition{
Type: fluxmeta.ReadyCondition,
Reason: helmcontrollerv2.InstallSucceededReason,
Status: metav1.ConditionTrue,
})
helmRelease.Status.History = helmcontrollerv2.Snapshots{
{
Name: coreComponent.helmReleaseName,
FirstDeployed: metav1.Now(),
LastDeployed: metav1.Now(),
},
}
Expect(k8sClient.Status().Update(ctx, helmRelease)).To(Succeed())
}

By("Create Cluster API CoreProvider object")

coreProvider := &capioperator.CoreProvider{
ObjectMeta: metav1.ObjectMeta{
Name: "capi",
Namespace: utils.DefaultSystemNamespace,
Labels: map[string]string{
"helm.toolkit.fluxcd.io/name": coreComponents["capi"].helmReleaseName,
},
},
}
Expect(k8sClient.Create(ctx, coreProvider)).To(Succeed())

coreProvider.Status.Conditions = clusterv1.Conditions{
{
Type: clusterv1.ReadyCondition,
Status: "True",
LastTransitionTime: metav1.Now(),
},
}

Expect(k8sClient.Status().Update(ctx, coreProvider)).To(Succeed())

By("Reconciling the Management object again to ensure the components status is updated")

_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{
NamespacedName: client.ObjectKeyFromObject(mgmt),
})
Expect(err).NotTo(HaveOccurred())

Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(mgmt), mgmt)).To(Succeed())
Expect(mgmt.Status.Components).To(BeEquivalentTo(map[string]hmcmirantiscomv1alpha1.ComponentStatus{
hmcmirantiscomv1alpha1.CoreHMCName: {Success: true, Template: providerTemplateRequiredComponent},
hmcmirantiscomv1alpha1.CoreCAPIName: {Success: true, Template: providerTemplateRequiredComponent},
Expand All @@ -265,6 +355,10 @@ var _ = Describe("Management Controller", func() {
Eventually(func() bool {
return apierrors.IsNotFound(k8sClient.Get(ctx, client.ObjectKeyFromObject(providerTemplateRequired), &hmcmirantiscomv1alpha1.ProviderTemplate{}))
}).WithTimeout(timeout).WithPolling(interval).Should(BeTrue())

coreProvider.Finalizers = nil
Expect(k8sClient.Update(ctx, coreProvider)).To(Succeed())
Expect(k8sClient.Delete(ctx, coreProvider)).To(Succeed())
})
})
})
3 changes: 3 additions & 0 deletions internal/controller/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
capioperator "sigs.k8s.io/cluster-api-operator/api/v1alpha2"
utilyaml "sigs.k8s.io/cluster-api/util/yaml"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -118,6 +119,8 @@ var _ = BeforeSuite(func() {
Expect(err).NotTo(HaveOccurred())
err = sveltosv1beta1.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())
err = capioperator.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())

// +kubebuilder:scaffold:scheme

Expand Down

0 comments on commit 10e2719

Please sign in to comment.