Skip to content

Commit

Permalink
HCP: validate HCC for minimumKubeletVersion
Browse files Browse the repository at this point in the history
by adding feature gate controller, using that to set oldestKubeletVersion
and gating updates to minimumKubeletVersion on that field

Signed-off-by: Peter Hunt <[email protected]>
  • Loading branch information
haircommander committed Nov 5, 2024
1 parent b43906d commit b506416
Show file tree
Hide file tree
Showing 7 changed files with 269 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
"github.com/aws/aws-sdk-go/service/kms"
"github.com/blang/semver/v4"
"github.com/go-logr/logr"
routev1 "github.com/openshift/api/route/v1"
hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
Expand Down Expand Up @@ -88,6 +89,7 @@ import (
"github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/reference"
"github.com/openshift/hypershift/support/upsert"
"github.com/openshift/hypershift/support/util"
nodelib "github.com/openshift/library-go/pkg/config/node"
prometheusoperatorv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
Expand Down Expand Up @@ -387,7 +389,25 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
Type: string(hyperv1.ValidHostedControlPlaneConfiguration),
ObservedGeneration: hostedControlPlane.Generation,
}
if err := r.validateConfigAndClusterCapabilities(hostedControlPlane); err != nil {
if err := r.validateConfigAndClusterCapabilities(ctx, hostedControlPlane); err != nil {
condition.Status = metav1.ConditionFalse
condition.Message = err.Error()
condition.Reason = hyperv1.InsufficientClusterCapabilitiesReason
} else {
condition.Status = metav1.ConditionTrue
condition.Message = "Configuration passes validation"
condition.Reason = hyperv1.AsExpectedReason
}
meta.SetStatusCondition(&hostedControlPlane.Status.Conditions, condition)
}

// Reconcile HostedClusterConfiguration
{
condition := metav1.Condition{
Type: string(hyperv1.ValidHostedControlPlaneConfiguration),
ObservedGeneration: hostedControlPlane.Generation,
}
if err := r.validateHostedClusterConfiguration(ctx, hostedControlPlane); err != nil {
condition.Status = metav1.ConditionFalse
condition.Message = err.Error()
condition.Reason = hyperv1.InsufficientClusterCapabilitiesReason
Expand Down Expand Up @@ -840,7 +860,7 @@ func healthCheckKASEndpoint(ingressPoint string, port int) error {
return nil
}

func (r *HostedControlPlaneReconciler) validateConfigAndClusterCapabilities(hc *hyperv1.HostedControlPlane) error {
func (r *HostedControlPlaneReconciler) validateConfigAndClusterCapabilities(ctx context.Context, hc *hyperv1.HostedControlPlane) error {
for _, svc := range hc.Spec.Services {
if svc.Type == hyperv1.Route && !r.ManagementClusterCapabilities.Has(capabilities.CapabilityRoute) {
return fmt.Errorf("cluster does not support Routes, but service %q is exposed via a Route", svc.Service)
Expand All @@ -849,6 +869,35 @@ func (r *HostedControlPlaneReconciler) validateConfigAndClusterCapabilities(hc *
return nil
}

func (r *HostedControlPlaneReconciler) validateHostedClusterConfiguration(ctx context.Context, hc *hyperv1.HostedControlPlane) error {
if hc.Spec.Configuration == nil || hc.Spec.Configuration.Node == nil {
return nil
}
if minVersion := hc.Spec.Configuration.Node.MinimumKubeletVersion; minVersion != "" {
if hc.Status.OldestKubeletVersion == nil || *hc.Status.OldestKubeletVersion == "" {
// No Kubelets are running in the cluster yet, this is safe?
return nil
}
v, err := semver.Parse(minVersion)
if err != nil {
return fmt.Errorf("invalid minimumKubeletVersion: %s %v", minVersion, err)
}
// hacky, but we don't have a node object, instead we have the oldest version passed
// from the status. We can just make a dummy node object, as we know what fields this
// function uses.
if _, err := nodelib.IsKubeletVersionTooOld(&corev1.Node{
Status: corev1.NodeStatus{
NodeInfo: corev1.NodeSystemInfo{
KubeletVersion: *hc.Status.OldestKubeletVersion,
},
},
}, &v); err != nil {
return fmt.Errorf("validating failed for %s: %v", minVersion, err)
}
}
return nil
}

func (r *HostedControlPlaneReconciler) LookupReleaseImage(ctx context.Context, hcp *hyperv1.HostedControlPlane) (*releaseinfo.ReleaseImage, error) {
pullSecret := common.PullSecret(hcp.Namespace)
if err := r.Client.Get(ctx, client.ObjectKeyFromObject(pullSecret), pullSecret); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1735,3 +1735,86 @@ func componentsFakeDependencies(componentName string, namespace string) []client

return []client.Object{fakeComponent}
}

func TestReconcileMinimumKubeletVersion(t *testing.T) {
testsCases := []struct {
name string
hc *hyperv1.HostedControlPlane
errMsg string
}{
{
name: "should not error if config is nil",
hc: &hyperv1.HostedControlPlane{
Spec: hyperv1.HostedControlPlaneSpec{},
},
},
{
name: "should not error if Node object is nil",
hc: &hyperv1.HostedControlPlane{
Spec: hyperv1.HostedControlPlaneSpec{
Configuration: &hyperv1.ClusterConfiguration{},
},
},
},
{
name: "should not error if status has empty OldestKubeletVersion",
hc: &hyperv1.HostedControlPlane{
Spec: hyperv1.HostedControlPlaneSpec{
Configuration: &hyperv1.ClusterConfiguration{
Node: &configv1.NodeSpec{
MinimumKubeletVersion: "1.30.0",
},
},
},
},
},
{
name: "should error if status has OldestKubeletVersion too old",
hc: &hyperv1.HostedControlPlane{
Spec: hyperv1.HostedControlPlaneSpec{
Configuration: &hyperv1.ClusterConfiguration{
Node: &configv1.NodeSpec{
MinimumKubeletVersion: "1.30.0",
},
},
},
Status: hyperv1.HostedControlPlaneStatus{
OldestKubeletVersion: ptr.To("1.29.0"),
},
},
errMsg: `validating failed for 1.30.0: kubelet version of node is 1.29.0, which is lower than minimumKubeletVersion of 1.30.0`,
},
{
name: "should not error if client returns all node new enough",
hc: &hyperv1.HostedControlPlane{
Spec: hyperv1.HostedControlPlaneSpec{
Configuration: &hyperv1.ClusterConfiguration{
Node: &configv1.NodeSpec{
MinimumKubeletVersion: "1.30.0",
},
},
},
Status: hyperv1.HostedControlPlaneStatus{
OldestKubeletVersion: ptr.To("1.30.0"),
},
},
errMsg: "",
},
}
for _, tc := range testsCases {
t.Run(tc.name, func(t *testing.T) {
g := NewGomegaWithT(t)

r := &HostedControlPlaneReconciler{
Log: ctrl.LoggerFrom(context.TODO()),
}

err := r.validateHostedClusterConfiguration(context.Background(), tc.hc)
if tc.errMsg == "" {
g.Expect(err).NotTo(HaveOccurred())
} else {
g.Expect(err.Error()).To(Equal(tc.errMsg))
}
})
}
}
24 changes: 12 additions & 12 deletions control-plane-operator/hostedclusterconfigoperator/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/configmetrics"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/cmca"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/drainer"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/featuregate"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/machine"
Expand All @@ -36,14 +37,12 @@ import (
"github.com/openshift/hypershift/support/releaseinfo"
"github.com/openshift/hypershift/support/upsert"
"github.com/openshift/hypershift/support/util"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/spf13/cobra"
"go.uber.org/zap/zapcore"

"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/cluster"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)
Expand All @@ -58,14 +57,15 @@ func NewCommand() *cobra.Command {
}

var controllerFuncs = map[string]operator.ControllerSetupFunc{
"controller-manager-ca": cmca.Setup,
resources.ControllerName: resources.Setup,
"inplaceupgrader": inplaceupgrader.Setup,
"node": node.Setup,
nodecount.ControllerName: nodecount.Setup,
"machine": machine.Setup,
"drainer": drainer.Setup,
hcpstatus.ControllerName: hcpstatus.Setup,
"controller-manager-ca": cmca.Setup,
resources.ControllerName: resources.Setup,
"inplaceupgrader": inplaceupgrader.Setup,
"node": node.Setup,
nodecount.ControllerName: nodecount.Setup,
featuregate.ControllerName: featuregate.Setup,
"machine": machine.Setup,
"drainer": drainer.Setup,
hcpstatus.ControllerName: hcpstatus.Setup,
}

type HostedClusterConfigOperator struct {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package featuregate

import (
"context"
"fmt"

"github.com/blang/semver/v4"
hypershiftv1beta1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
"github.com/openshift/hypershift/support/util"
nodelib "github.com/openshift/library-go/pkg/config/node"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

hypershiftv1beta1applyconfigurations "github.com/openshift/hypershift/client/applyconfiguration/hypershift/v1beta1"
hypershiftclient "github.com/openshift/hypershift/client/clientset/clientset"
)

type minimumKubeletVersionReconciler struct {
hcpName, hcpNamespace string
client hypershiftclient.Interface
lister client.Client

guestClusterClient client.Client
}

func (r *minimumKubeletVersionReconciler) Reconcile(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.Info("Reconciling")

var hcp hypershiftv1beta1.HostedControlPlane
if err := r.lister.Get(ctx, client.ObjectKey{
Namespace: r.hcpNamespace,
Name: r.hcpName,
}, &hcp); err != nil {
return reconcile.Result{}, err
}
if isPaused, duration := util.IsReconciliationPaused(log, hcp.Spec.PausedUntil); isPaused {
log.Info("Reconciliation paused", "pausedUntil", *hcp.Spec.PausedUntil)
return ctrl.Result{
RequeueAfter: duration,
}, nil
}
if hcp.ObjectMeta.DeletionTimestamp != nil {
return reconcile.Result{}, nil
}

nodes := &corev1.NodeList{}
if err := r.guestClusterClient.List(ctx, nodes); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to get Nodes: %w", err)
}

currentOldestKubelet := getOldestKubeletVersion(nodes.Items)
if currentOldestKubelet == nil {
// no valid nodes, leave the field unset
return reconcile.Result{}, nil
}

cfg := hypershiftv1beta1applyconfigurations.HostedControlPlane(r.hcpName, r.hcpNamespace)
cfg.Status = hypershiftv1beta1applyconfigurations.HostedControlPlaneStatus().WithOldestKubeletVersion(currentOldestKubelet.String())
_, err := r.client.HypershiftV1beta1().HostedControlPlanes(r.hcpNamespace).ApplyStatus(ctx, cfg, metav1.ApplyOptions{FieldManager: ControllerName})
return reconcile.Result{}, err
}

func getOldestKubeletVersion(nodes []corev1.Node) *semver.Version {
var oldestVersion *semver.Version
for _, node := range nodes {
v, err := nodelib.ParseKubeletVersion(&node)
if err != nil {
continue
}
if oldestVersion == nil || v.LT(*oldestVersion) {
oldestVersion = v
}
}
return oldestVersion
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package featuregate

import (
"context"
"time"

hypershiftclient "github.com/openshift/hypershift/client/clientset/clientset"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/operator"
featuregate "github.com/openshift/hypershift/hypershift-operator/featuregate"
corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/util/workqueue"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

const ControllerName = "featuregate"

func Setup(ctx context.Context, opts *operator.HostedClusterConfigOperatorConfig) error {
if !featuregate.Gates.Enabled(featuregate.MinimumKubeletVersion) {
return nil
}

hypershiftClient, err := hypershiftclient.NewForConfig(opts.CPCluster.GetConfig())
if err != nil {
return err
}

return ctrl.NewControllerManagedBy(opts.Manager).
Named(ControllerName).
For(&corev1.Node{}).
WithOptions(controller.Options{
RateLimiter: workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](1*time.Second, 10*time.Second),
}).Complete(&minimumKubeletVersionReconciler{
hcpName: opts.HCPName,
hcpNamespace: opts.Namespace,
client: hypershiftClient,
lister: opts.CPCluster.GetClient(),
guestClusterClient: opts.Manager.GetClient(),
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func Setup(ctx context.Context, opts *operator.HostedClusterConfigOperatorConfig
if err != nil {
return err
}

return ctrl.NewControllerManagedBy(opts.Manager).
Named(ControllerName).
For(&corev1.Node{}).
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ require (
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5
github.com/aws/aws-sdk-go v1.52.6
github.com/blang/semver v3.5.1+incompatible
github.com/blang/semver/v4 v4.0.0
github.com/clarketm/json v1.17.1
github.com/coreos/ignition/v2 v2.17.0
github.com/distribution/reference v0.6.0
Expand Down Expand Up @@ -117,7 +118,6 @@ require (
github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/blang/semver/v4 v4.0.0 // indirect
github.com/cenkalti/backoff/v4 v4.3.0 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/chai2010/gettext-go v1.0.3 // indirect
Expand Down

0 comments on commit b506416

Please sign in to comment.