From c9fbb0052909b6c769ab1e13322f515c934faa7c Mon Sep 17 00:00:00 2001 From: Alan Conway Date: Wed, 6 Dec 2023 10:29:31 -0400 Subject: [PATCH] LOG-4445: Forwarder must use hosted clusterID on HCP cluster. NOTE: Using unstructured objects for HCP because of dependency problems with the hypershift API package. See https://issues.redhat.com/browse/HOSTEDCP-336. --- .../clusterlogging_controller.go | 14 ++- .../hostedcontrolplane/hostedcontrolplane.go | 56 ++++++++++++ .../hostedcontrolplane_test.go | 85 +++++++++++++++++++ internal/hostedcontrolplane/suite_test.go | 13 +++ 4 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 internal/hostedcontrolplane/hostedcontrolplane.go create mode 100644 internal/hostedcontrolplane/hostedcontrolplane_test.go create mode 100644 internal/hostedcontrolplane/suite_test.go diff --git a/controllers/clusterlogging/clusterlogging_controller.go b/controllers/clusterlogging/clusterlogging_controller.go index 3c1519e348..c45f6f67df 100644 --- a/controllers/clusterlogging/clusterlogging_controller.go +++ b/controllers/clusterlogging/clusterlogging_controller.go @@ -3,6 +3,7 @@ package clusterlogging import ( "context" + "github.com/openshift/cluster-logging-operator/internal/hostedcontrolplane" "github.com/openshift/cluster-logging-operator/internal/logstore/lokistack" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -114,8 +115,9 @@ func (r *ReconcileClusterLogging) Reconcile(ctx context.Context, request ctrl.Re return ctrl.Result{}, err } + clusterVersion, clusterID := r.GetClusterVersionID(ctx, request.Namespace) resourceNames := factory.GenerateResourceNames(clf) - if err = k8shandler.Reconcile(&instance, &clf, r.Client, r.Reader, r.Recorder, r.ClusterVersion, r.ClusterID, resourceNames); err != nil { + if err = k8shandler.Reconcile(&instance, &clf, r.Client, r.Reader, r.Recorder, clusterVersion, clusterID, resourceNames); err != nil { telemetry.Data.CLInfo.Set("healthStatus", constants.UnHealthyStatus) log.Error(err, "Error reconciling clusterlogging instance") instance.Status.Conditions.SetCondition(loggingv1.CondInvalid("error reconciling clusterlogging instance: %v", err)) @@ -187,3 +189,13 @@ func (r *ReconcileClusterLogging) SetupWithManager(mgr ctrl.Manager) error { return controllerBuilder.Complete(r) } + +func (r *ReconcileClusterLogging) GetClusterVersionID(ctx context.Context, namespace string) (version, id string) { + // If reconciling in a hosted control plane namespace, use the guest cluster version/id + // provided by the hostedcontrolplane resource. + if info := hostedcontrolplane.GetVersionID(ctx, r.Client, namespace); info != nil { + return info.Version, info.ID + } + // Otherwise use the cluster-ID established at start-up. + return r.ClusterVersion, r.ClusterID +} diff --git a/internal/hostedcontrolplane/hostedcontrolplane.go b/internal/hostedcontrolplane/hostedcontrolplane.go new file mode 100644 index 0000000000..a004831c93 --- /dev/null +++ b/internal/hostedcontrolplane/hostedcontrolplane.go @@ -0,0 +1,56 @@ +package hostedcontrolplane + +import ( + "context" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + client "sigs.k8s.io/controller-runtime/pkg/client" +) + +// VersionID contains the version and ID of a HostedControlPlane +type VersionID struct { + Version string // Version of the guest cluster. + ID string // Unique identifier of the guest cluster. +} + +// GetVersionID returns the guest cluster version and ID for a HCP namepace. +// Returns nil if the namespace is not a valid HCP namespace with a non-empty version and ID. +func GetVersionID(ctx context.Context, c client.Reader, namespace string) *VersionID { + // TODO: Using unstructured objects for HCP because of dependency problems with the hypershift API package. + // See https://issues.redhat.com/browse/HOSTEDCP-336 + l := &unstructured.UnstructuredList{} + l.SetGroupVersionKind(hcpGVK) + err := c.List(ctx, l, client.InNamespace(namespace)) + if err != nil || len(l.Items) != 1 { + // A valid HCP namespace must contain exactly one HCP instance, anything else is invalid. + return nil + } + id := dig(l.Items[0].Object, "spec", "clusterID") + version := dig(l.Items[0].Object, "status", "versionStatus", "desired", "version") + if version == "" { + // Check deprecated top-level version field, see: + // https://github.com/openshift/hypershift/blob//api/hypershift/v1beta1/hosted_controlplane.go#L241 + version = dig(l.Items[0].Object, "status", "version") + } + if id == "" || version == "" { + return nil + } + return &VersionID{Version: version, ID: id} +} + +var hcpGVK = schema.GroupVersionKind{ + Group: "hypershift.openshift.io", + Version: "v1beta1", + Kind: "HostedControlPlane", +} + +// dig out a string value from nested map[string]any +func dig(v any, keys ...string) string { + for _, k := range keys { + m, _ := v.(map[string]any) + v = m[k] // Index of nil map returns nil. + } + s, _ := v.(string) + return s +} diff --git a/internal/hostedcontrolplane/hostedcontrolplane_test.go b/internal/hostedcontrolplane/hostedcontrolplane_test.go new file mode 100644 index 0000000000..60999ce101 --- /dev/null +++ b/internal/hostedcontrolplane/hostedcontrolplane_test.go @@ -0,0 +1,85 @@ +package hostedcontrolplane + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + configv1 "github.com/openshift/api/config/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +var _ = Describe("[internal][hostedcontrolplane]", func() { + runtime.Must(configv1.AddToScheme(scheme.Scheme)) + version, id := "v1.2.3", "1234-abcd" + + Describe("GetVersionID", func() { + It("gets version and ID for a HCP namespace", func() { + hcp := &unstructured.Unstructured{} + hcp.Object = map[string]any{ + "spec": map[string]any{ + "clusterID": id, + }, + "status": map[string]any{ + "versionStatus": map[string]any{ + "desired": map[string]any{ //configv1.Release{ + "version": version, + }, + }, + }, + } + hcp.SetGroupVersionKind(hcpGVK) + hcp.SetName("foobar") + hcp.SetNamespace("testing") + c := fake.NewClientBuilder().WithObjects(hcp).Build() + info := GetVersionID(context.Background(), c, hcp.GetNamespace()) + Expect(info).NotTo(BeNil()) + Expect(info).To(Equal(&VersionID{Version: version, ID: id})) + }) + + It("gets version and ID for a HCP namespace using deprecated version field", func() { + hcp := &unstructured.Unstructured{} + hcp.Object = map[string]any{ + "spec": map[string]any{ + "clusterID": id, + }, + "status": map[string]any{ + "version": version, + }, + } + hcp.SetGroupVersionKind(hcpGVK) + hcp.SetName("foobar") + hcp.SetNamespace("testing") + c := fake.NewClientBuilder().WithObjects(hcp).Build() + info := GetVersionID(context.Background(), c, hcp.GetNamespace()) + Expect(info).NotTo(BeNil()) + Expect(info).To(Equal(&VersionID{Version: version, ID: id})) + }) + + It("returns nil for non-HCP namespace", func() { + c := fake.NewClientBuilder().Build() + info := GetVersionID(context.Background(), c, "testing") + Expect(info).To(BeNil()) + }) + + It("returns nil for an invalid HCP object (lacking version and ID)", func() { + hcp := &unstructured.Unstructured{} + hcp.Object = map[string]any{ + "spec": map[string]any{ + "clusterID": map[string]any{"foo": "bar"}, // Invalid ID + }, + "status": map[string]any{}, // Missing version + } + hcp.SetGroupVersionKind(hcpGVK) + hcp.SetName("foobar") + hcp.SetNamespace("testing") + c := fake.NewClientBuilder().WithObjects(hcp).Build() + info := GetVersionID(context.Background(), c, hcp.GetNamespace()) + Expect(info).To(BeNil()) + }) + + }) +}) diff --git a/internal/hostedcontrolplane/suite_test.go b/internal/hostedcontrolplane/suite_test.go new file mode 100644 index 0000000000..a033a3c430 --- /dev/null +++ b/internal/hostedcontrolplane/suite_test.go @@ -0,0 +1,13 @@ +package hostedcontrolplane + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestV1Logging(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "[internal][clusterinfo] Suite") +}