From ee5e881bdf35a456587d4fd10b1c1433ba5fad66 Mon Sep 17 00:00:00 2001 From: Simon Gerber Date: Tue, 12 Nov 2024 16:50:27 +0100 Subject: [PATCH 1/2] Add controller which reports K8s and OCP version to control-api The controller reconciles clusterversion.config.openshift.io which should be sufficient to detect changes in the K8s/OCP version. --- controllers/zonek8sversion_controller.go | 138 +++++++++++++++++++++++ main.go | 37 +++++- 2 files changed, 170 insertions(+), 5 deletions(-) create mode 100644 controllers/zonek8sversion_controller.go diff --git a/controllers/zonek8sversion_controller.go b/controllers/zonek8sversion_controller.go new file mode 100644 index 0000000..e8275fd --- /dev/null +++ b/controllers/zonek8sversion_controller.go @@ -0,0 +1,138 @@ +package controllers + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "time" + + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/version" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/record" + + controlv1 "github.com/appuio/control-api/apis/v1" + configv1 "github.com/openshift/api/config/v1" + + "go.uber.org/multierr" +) + +type ZoneK8sVersionReconciler struct { + client.Client + Scheme *runtime.Scheme + Recorder record.EventRecorder + + ForeignClient client.Client + RESTClient rest.Interface + + // upstream zone ID. The agent expects that the control-api zone + // object is labeled with + ZoneID string +} + +const ( + upstreamZoneIdentifierLabelKey = "control.appuio.io/zone-cluster-id" + kubernetesVersionFeatureKey = "kubernetesVersion" + openshiftVersionFeatureKey = "openshiftVersion" +) + +// Reconcile reads the K8s and OCP versions and writes them to the upstream +// zone +// The logic in this reconcile function is adapted from +// https://github.com/projectsyn/steward +func (r *ZoneK8sVersionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + l := log.FromContext(ctx) + l.Info("Reconciling zone K8s version") + + var cv = configv1.ClusterVersion{} + if err := r.Client.Get(ctx, req.NamespacedName, &cv); err != nil { + return ctrl.Result{}, err + } + + ocpVersion, err := extractOpenShiftVersion(&cv) + if err != nil { + return ctrl.Result{}, err + } + + l.Info("OCP current version", "version", ocpVersion) + + // We don't use client-go's ServerVersion() so we get context support + body, err := r.RESTClient.Get().AbsPath("/version").Do(ctx).Raw() + if err != nil { + return ctrl.Result{}, err + } + var k8sVersion version.Info + err = json.Unmarshal(body, &k8sVersion) + if err != nil { + return ctrl.Result{}, err + } + l.Info("K8s current version", "version", k8sVersion) + + // List zones by label because we don't enforce any naming conventions + // for the Zone objects on the control-api cluster. + var zones = controlv1.ZoneList{} + if err := r.ForeignClient.List(ctx, &zones, client.MatchingLabels{upstreamZoneIdentifierLabelKey: r.ZoneID}); err != nil { + return ctrl.Result{}, err + } + + if len(zones.Items) == 0 { + l.Info("No upstream zone found", "zone ID", r.ZoneID) + return ctrl.Result{}, nil + } + + if len(zones.Items) > 1 { + l.Info("Multiple upstream zones found, updating all", "zone ID", r.ZoneID) + } + + var errs []error + for _, z := range zones.Items { + z.Data.Features[kubernetesVersionFeatureKey] = + fmt.Sprintf("%s.%s", k8sVersion.Major, k8sVersion.Minor) + z.Data.Features[openshiftVersionFeatureKey] = + fmt.Sprintf("%s.%s", ocpVersion.Major, ocpVersion.Minor) + if err := r.ForeignClient.Update(ctx, &z); err != nil { + errs = append(errs, err) + } + } + + return ctrl.Result{}, multierr.Combine(errs...) +} + +// SetupWithManager sets up the controller with the Manager. +func (r *ZoneK8sVersionReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&configv1.ClusterVersion{}). + Named("zone_k8s_version"). + Complete(r) +} + +// extract version of latest completed and verified upgrade from the OCP ClusterVersion resource. +func extractOpenShiftVersion(cv *configv1.ClusterVersion) (*version.Info, error) { + currentVersion := "" + lastUpdate := time.Time{} + for _, h := range cv.Status.History { + if h.State == "Completed" && h.Verified == true && h.CompletionTime.Time.After(lastUpdate) { + currentVersion = h.Version + lastUpdate = h.CompletionTime.Time + } + } + if currentVersion == "" { + currentVersion = cv.Status.Desired.Version + } + + if currentVersion == "" { + return nil, fmt.Errorf("Unable to extract current OpenShift version") + } + + verparts := strings.Split(currentVersion, ".") + version := version.Info{ + Major: verparts[0], + Minor: verparts[1], + } + return &version, nil +} diff --git a/main.go b/main.go index 7c1c254..1fdea34 100644 --- a/main.go +++ b/main.go @@ -15,6 +15,7 @@ import ( authenticationv1 "k8s.io/api/authentication/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/kubernetes" clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cluster" @@ -33,6 +34,8 @@ import ( "github.com/appuio/appuio-cloud-agent/skipper" "github.com/appuio/appuio-cloud-agent/webhooks" whoamicli "github.com/appuio/appuio-cloud-agent/whoami" + + configv1 "github.com/openshift/api/config/v1" ) var ( @@ -56,6 +59,7 @@ func init() { utilruntime.Must(projectv1.AddToScheme(scheme)) utilruntime.Must(agentv1.AddToScheme(scheme)) utilruntime.Must(controlv1.AddToScheme(scheme)) + utilruntime.Must(configv1.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme } @@ -77,7 +81,7 @@ func main() { flag.StringVar(&controlAPIURL, "control-api-url", "", "URL of the control API. If set agent does not use `-kubeconfig-control-api`. Expects a bearer token in `CONTROL_API_BEARER_TOKEN` env var.") var upstreamZoneIdentifier string - flag.StringVar(&upstreamZoneIdentifier, "upstream-zone-identifier", "", "Identifies the agent in the control API. Currently used for Team/OrganizationMembers finalizer. Must be set if the GroupSync controller is enabled.") + flag.StringVar(&upstreamZoneIdentifier, "upstream-zone-identifier", "", "Identifies the agent in the control API. Currently used for Team/OrganizationMembers finalizer and the K8s version reporting.") var selectedUsageProfile string flag.StringVar(&selectedUsageProfile, "usage-profile", "", "UsageProfile to use. Applies all profiles if empty. Dynamic selection is not supported yet.") @@ -175,8 +179,14 @@ func main() { os.Exit(1) } + if upstreamZoneIdentifier == "" { + setupLog.Error(err, "upstream-zone-identifier must be set.") + os.Exit(1) + } + registerRatioController(mgr, conf, conf.OrganizationLabel) registerOrganizationRBACController(mgr, conf.OrganizationLabel, conf.DefaultOrganizationClusterRoles) + registerZoneK8sVersionController(mgr, controlAPICluster, upstreamZoneIdentifier) if !disableUserAttributeSync { if err := (&controllers.UserAttributeSyncReconciler{ @@ -191,10 +201,6 @@ func main() { } } if !disableGroupSync { - if upstreamZoneIdentifier == "" { - setupLog.Error(err, "upstream-zone-identifier must be set if GroupSync controller is enabled") - os.Exit(1) - } if err := (&controllers.GroupSyncReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), @@ -452,3 +458,24 @@ func registerRatioController(mgr ctrl.Manager, conf Config, orgLabel string) { os.Exit(1) } } + +func registerZoneK8sVersionController(mgr ctrl.Manager, controlAPICluster cluster.Cluster, upstreamZoneIdentifier string) { + restclient, err := kubernetes.NewForConfig(mgr.GetConfig()) + if err != nil { + setupLog.Error(err, "unable to create clientset for config", "controller", "zone-k8s-version") + os.Exit(1) + } + if err := (&controllers.ZoneK8sVersionReconciler{ + Client: mgr.GetClient(), + RESTClient: restclient.RESTClient(), + Recorder: mgr.GetEventRecorderFor("zone-k8s-version-controller"), + Scheme: mgr.GetScheme(), + + ForeignClient: controlAPICluster.GetClient(), + + ZoneID: upstreamZoneIdentifier, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "zone-k8s-version") + os.Exit(1) + } +} From e5a51727244a0aef9bedbf16a404ccb1f1d87834 Mon Sep 17 00:00:00 2001 From: Simon Gerber Date: Wed, 13 Nov 2024 14:49:39 +0100 Subject: [PATCH 2/2] Add tests for ZoneK8sVersion reconciler --- controllers/utils_test.go | 2 + controllers/zonek8sversion_controller_test.go | 175 ++++++++++++++++++ 2 files changed, 177 insertions(+) create mode 100644 controllers/zonek8sversion_controller_test.go diff --git a/controllers/utils_test.go b/controllers/utils_test.go index bc57ec9..69b6d05 100644 --- a/controllers/utils_test.go +++ b/controllers/utils_test.go @@ -5,6 +5,7 @@ import ( "testing" controlv1 "github.com/appuio/control-api/apis/v1" + configv1 "github.com/openshift/api/config/v1" userv1 "github.com/openshift/api/user/v1" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -43,6 +44,7 @@ func prepareClient(t *testing.T, initObjs ...client.Object) (client.WithWatch, * require.NoError(t, cloudagentv1.AddToScheme(scheme)) require.NoError(t, controlv1.AddToScheme(scheme)) require.NoError(t, userv1.AddToScheme(scheme)) + require.NoError(t, configv1.AddToScheme(scheme)) client := fake.NewClientBuilder(). WithScheme(scheme). diff --git a/controllers/zonek8sversion_controller_test.go b/controllers/zonek8sversion_controller_test.go new file mode 100644 index 0000000..dedc537 --- /dev/null +++ b/controllers/zonek8sversion_controller_test.go @@ -0,0 +1,175 @@ +package controllers + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "testing" + "time" + + controlv1 "github.com/appuio/control-api/apis/v1" + configv1 "github.com/openshift/api/config/v1" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + //"k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/version" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + + restfake "k8s.io/client-go/rest/fake" +) + +func Test_ZoneK8sVersionReconciler_Reconcile(t *testing.T) { + zoneID := "c-appuio-test-cluster" + zone := controlv1.Zone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{ + upstreamZoneIdentifierLabelKey: zoneID, + }, + }, + Data: controlv1.ZoneData{ + Features: map[string]string{ + "foo": "bar", + }, + }, + } + otherZone := controlv1.Zone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-2", + Labels: map[string]string{ + upstreamZoneIdentifierLabelKey: "c-appuio-other-cluster", + }, + }, + Data: controlv1.ZoneData{ + Features: map[string]string{ + "foo": "bar", + }, + }, + } + foreignClient, _, _ := prepareClient(t, &zone, &otherZone) + cv := makeClusterVersion("4.16.19", []configv1.UpdateHistory{}) + client, scheme, recorder := prepareClient(t, &cv) + + version := version.Info{ + Major: "1", + Minor: "29", + } + marshaledVersion, err := json.Marshal(version) + require.NoError(t, err) + + // Setup a fake REST client which returns the marshaled version.Info + // on requests on /version + restclient := restfake.RESTClient{ + NegotiatedSerializer: serializer.WithoutConversionCodecFactory{CodecFactory: clientgoscheme.Codecs}, + Client: restfake.CreateHTTPClient( + func(req *http.Request) (*http.Response, error) { + if req.Method == "GET" && req.URL.Path == "/version" { + resp := http.Response{ + Header: make(http.Header, 0), + Body: io.NopCloser(bytes.NewBuffer(marshaledVersion)), + ContentLength: int64(len(marshaledVersion)), + Status: "200 OK", + StatusCode: 200, + Proto: "HTTP/1.1", + ProtoMajor: 1, + ProtoMinor: 1, + Request: req, + } + resp.Header.Add("Content-Type", "application/json; charset=utf-8") + return &resp, nil + } + return nil, fmt.Errorf("Unexpected request") + }, + ), + } + + subject := ZoneK8sVersionReconciler{ + Client: client, + Scheme: scheme, + Recorder: recorder, + ForeignClient: foreignClient, + RESTClient: &restclient, + ZoneID: zoneID, + } + + // NOTE(sg): ClusterVersion is a singleton which is always named + // version + _, err = subject.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: "version"}}) + require.NoError(t, err) + + // Get updated zone from the foreign client to check added fields + updatedZone := controlv1.Zone{} + err = foreignClient.Get(context.Background(), types.NamespacedName{Name: "test"}, &updatedZone) + require.NoError(t, err) + require.Equal(t, "4.16", updatedZone.Data.Features[openshiftVersionFeatureKey], "OCP version is set") + require.Equal(t, "1.29", updatedZone.Data.Features[kubernetesVersionFeatureKey], "K8s version is set") + require.Equal(t, "bar", updatedZone.Data.Features["foo"], "Unrelated fields are left in place") + + // Verify that unrelated zone isn't updated + updatedOtherZone := controlv1.Zone{} + err = foreignClient.Get(context.Background(), types.NamespacedName{Name: "test-2"}, &updatedOtherZone) + require.NoError(t, err) + require.Equal(t, controlv1.Features{"foo": "bar"}, updatedOtherZone.Data.Features, "unrelated zones are untouched") +} + +func Test_extractOpenShiftVersion(t *testing.T) { + cv := makeClusterVersion("4.16.5", []configv1.UpdateHistory{}) + v, err := extractOpenShiftVersion(&cv) + require.NoError(t, err) + require.Equal(t, "4", v.Major) + require.Equal(t, "16", v.Minor) +} + +func Test_extractOpenShiftVersionWithHistory(t *testing.T) { + history := []configv1.UpdateHistory{ + configv1.UpdateHistory{ + State: "Partial", + StartedTime: metav1.Time{Time: time.Now().Add(-1 * time.Hour)}, + CompletionTime: nil, + Version: "4.16.5", + }, + configv1.UpdateHistory{ + State: "Completed", + StartedTime: metav1.Time{Time: time.Now().Add(-3 * time.Hour)}, + CompletionTime: &metav1.Time{Time: time.Now().Add(-2 * time.Hour)}, + Version: "4.15.25", + Verified: true, + }, + configv1.UpdateHistory{ + State: "Completed", + StartedTime: metav1.Time{Time: time.Now().Add(-24 * time.Hour)}, + CompletionTime: &metav1.Time{Time: time.Now().Add(-23 * time.Hour)}, + Version: "4.14.29", + Verified: true, + }, + } + cv := makeClusterVersion("4.16.5", history) + v, err := extractOpenShiftVersion(&cv) + require.NoError(t, err) + require.Equal(t, "4", v.Major) + require.Equal(t, "15", v.Minor, "Prefer completed upgrade in history over desired upgrade") +} + +func makeClusterVersion(desired string, history []configv1.UpdateHistory) configv1.ClusterVersion { + return configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: "ocpID", + Channel: "stable-4.16", + }, + Status: configv1.ClusterVersionStatus{ + Desired: configv1.Release{ + Version: desired, + }, + History: history, + }, + } +}