Skip to content

Commit

Permalink
Reflect available upgrades in cluster status: addressed comments
Browse files Browse the repository at this point in the history
* Several watcher improvements: events, code enhancements
* Changed availableUpgrades type into []string
  • Loading branch information
eromanova committed Oct 28, 2024
1 parent c8508d4 commit ab3f033
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 34 deletions.
2 changes: 1 addition & 1 deletion api/v1alpha1/managedcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ type ManagedClusterStatus struct {
// AvailableUpgrades is the list of ClusterTemplate names to which
// this cluster can be upgraded. It can be an empty array, which means no upgrades are
// available.
AvailableUpgrades []AvailableUpgrade `json:"availableUpgrades,omitempty"`
AvailableUpgrades []string `json:"availableUpgrades,omitempty"`
// ObservedGeneration is the last observed generation.
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 15 additions & 10 deletions internal/controller/managedcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,19 @@ 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"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
capz "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
capv "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/yaml"

Expand Down Expand Up @@ -1027,9 +1029,9 @@ func (r *ManagedClusterReconciler) setAvailableUpgrades(ctx context.Context, man
}
}
}
availableUpgrades := make([]hmc.AvailableUpgrade, 0, len(availableUpgradesMap))
availableUpgrades := make([]string, 0, len(availableUpgradesMap))
for _, availableUpgrade := range availableUpgradesMap {
availableUpgrades = append(availableUpgrades, availableUpgrade)
availableUpgrades = append(availableUpgrades, availableUpgrade.Name)
}

managedCluster.Status.AvailableUpgrades = availableUpgrades
Expand Down Expand Up @@ -1060,23 +1062,22 @@ func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
).
Watches(&hmc.ClusterTemplateChain{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []ctrl.Request {
chain := &hmc.ClusterTemplateChain{}
err := r.Client.Get(ctx, types.NamespacedName{Namespace: o.GetNamespace(), Name: o.GetName()}, chain)
if err != nil {
return []ctrl.Request{}
chain, ok := o.(*hmc.ClusterTemplateChain)
if !ok {
return nil
}

var req []ctrl.Request
for _, template := range getTemplateNamesManagedByChain(chain) {
managedClusters := &hmc.ManagedClusterList{}
err = r.Client.List(ctx, managedClusters,
client.InNamespace(o.GetNamespace()),
err := r.Client.List(ctx, managedClusters,
client.InNamespace(chain.Namespace),
client.MatchingFields{hmc.TemplateKey: template})
if err != nil {
return []ctrl.Request{}
}
for _, cluster := range managedClusters.Items {
req = append(req, reconcile.Request{
req = append(req, ctrl.Request{
NamespacedName: client.ObjectKey{
Namespace: cluster.Namespace,
Name: cluster.Name,
Expand All @@ -1086,6 +1087,10 @@ func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
}
return req
}),
builder.WithPredicates(predicate.Funcs{
UpdateFunc: func(event.UpdateEvent) bool { return false },
GenericFunc: func(event.GenericEvent) bool { return false },
}),
).
Complete(r)
}
9 changes: 1 addition & 8 deletions internal/webhook/managedcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne
}

if oldTemplate != newTemplate {
isUpgradeAvailable := validateAvailableUpgrade(oldManagedCluster, newTemplate)
if !isUpgradeAvailable {
if !slices.Contains(oldManagedCluster.Status.AvailableUpgrades, newTemplate) {
msg := fmt.Sprintf("Cluster can't be upgraded from %s to %s. This upgrade sequence is not allowed", oldTemplate, newTemplate)
return admission.Warnings{msg}, errClusterUpgradeForbidden
}
Expand All @@ -123,12 +122,6 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne
return nil, nil
}

func validateAvailableUpgrade(oldManagedCluster *hmcv1alpha1.ManagedCluster, newTemplate string) bool {
return slices.ContainsFunc(oldManagedCluster.Status.AvailableUpgrades, func(au hmcv1alpha1.AvailableUpgrade) bool {
return newTemplate == au.Name
})
}

func validateK8sCompatibility(ctx context.Context, cl client.Client, template *hmcv1alpha1.ClusterTemplate, mc *hmcv1alpha1.ManagedCluster) error {
if len(mc.Spec.Services) == 0 || template.Status.KubernetesVersion == "" {
return nil // nothing to do
Expand Down
6 changes: 3 additions & 3 deletions internal/webhook/managedcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ func TestManagedClusterValidateUpdate(t *testing.T) {
name: "update spec.template: should fail if the new cluster template was found but is invalid (some validation error)",
oldManagedCluster: managedcluster.NewManagedCluster(
managedcluster.WithClusterTemplate(testTemplateName),
managedcluster.WithAvailableUpgrades([]v1alpha1.AvailableUpgrade{{Name: newTemplateName}}),
managedcluster.WithAvailableUpgrades([]string{newTemplateName}),
),
newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(newTemplateName)),
existingObjects: []runtime.Object{
Expand All @@ -302,7 +302,7 @@ func TestManagedClusterValidateUpdate(t *testing.T) {
oldManagedCluster: managedcluster.NewManagedCluster(
managedcluster.WithClusterTemplate(testTemplateName),
managedcluster.WithCredential(testCredentialName),
managedcluster.WithAvailableUpgrades([]v1alpha1.AvailableUpgrade{}),
managedcluster.WithAvailableUpgrades([]string{}),
),
newManagedCluster: managedcluster.NewManagedCluster(
managedcluster.WithClusterTemplate(upgradeTargetTemplateName),
Expand Down Expand Up @@ -337,7 +337,7 @@ func TestManagedClusterValidateUpdate(t *testing.T) {
oldManagedCluster: managedcluster.NewManagedCluster(
managedcluster.WithClusterTemplate(testTemplateName),
managedcluster.WithCredential(testCredentialName),
managedcluster.WithAvailableUpgrades([]v1alpha1.AvailableUpgrade{{Name: newTemplateName}}),
managedcluster.WithAvailableUpgrades([]string{newTemplateName}),
),
newManagedCluster: managedcluster.NewManagedCluster(
managedcluster.WithClusterTemplate(newTemplateName),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,7 @@ spec:
this cluster can be upgraded. It can be an empty array, which means no upgrades are
available.
items:
description: AvailableUpgrade is the definition of the available
upgrade for the Template
properties:
name:
description: Name is the name of the Template to which the upgrade
is available.
type: string
required:
- name
type: object
type: string
type: array
conditions:
description: Conditions contains details for the current state of
Expand Down
2 changes: 1 addition & 1 deletion test/objects/managedcluster/managedcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func WithCredential(credName string) Opt {
}
}

func WithAvailableUpgrades(availableUpgrades []v1alpha1.AvailableUpgrade) Opt {
func WithAvailableUpgrades(availableUpgrades []string) Opt {
return func(p *v1alpha1.ManagedCluster) {
p.Status.AvailableUpgrades = availableUpgrades
}
Expand Down

0 comments on commit ab3f033

Please sign in to comment.