Skip to content

Commit

Permalink
[ACM-16286]: Ensure MCO spec addon options are propagated correctly (s…
Browse files Browse the repository at this point in the history 
…tolostron#1718)

* [ACM-16286]: Ensure MCO spec addon options are propagated correctly

Signed-off-by: Saswata Mukherjee <[email protected]>

* [ACM-16286]: Update all observabilityaddon manifests

Signed-off-by: Saswata Mukherjee <[email protected]>

* [ACM-16286]: Ensure hub and spoke obsaddon are synced based on spec

Signed-off-by: Saswata Mukherjee <[email protected]>

* [ACM-16286]: Allow customizing ObsAddon in spoke ns

Signed-off-by: Saswata Mukherjee <[email protected]>

* [ACM-16286]: Comments

Signed-off-by: Saswata Mukherjee <[email protected]>

* Add addon source annotation

Signed-off-by: Saswata Mukherjee <[email protected]>

* Add handling for empty map

Signed-off-by: Saswata Mukherjee <[email protected]>

* Add handling for nil maps

Signed-off-by: Saswata Mukherjee <[email protected]>

* Add annotation predicate, update in mco mode

Signed-off-by: Saswata Mukherjee <[email protected]>

* Fix unit test, make addon annotation constant

Signed-off-by: Saswata Mukherjee <[email protected]>

* Update comment

Signed-off-by: Saswata Mukherjee <[email protected]>

* Address comments

Signed-off-by: Saswata Mukherjee <[email protected]>

* Use semantic deepequal, add test for setter

Signed-off-by: Saswata Mukherjee <[email protected]>

* Set spoke addon correctly after comparison

Signed-off-by: Saswata Mukherjee <[email protected]>

---------

Signed-off-by: Saswata Mukherjee <[email protected]>
  • Loading branch information
@saswatamcode
saswatamcode committed Dec 16, 2024
1 parent 0235766 commit 7db60ca
Show file tree
Hide file tree
Showing 7 changed files with 368 additions and 41 deletions.
99 changes: 87 additions & 12 deletions ...etrics/config/crd/bases/observability.open-cluster-management.io_observabilityaddons.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.4.1
controller-gen.kubebuilder.io/version: v0.14.0
creationTimestamp: null
name: observabilityaddons.observability.open-cluster-management.io
spec:
Expand All @@ -23,33 +22,109 @@ spec:
description: ObservabilityAddon is the Schema for the observabilityaddon API
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
description: |-
APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
description: |-
Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
type: string
metadata:
type: object
spec:
description: ObservabilityAddonSpec is the spec of observability addon
description: ObservabilityAddonSpec is the spec of observability addon.
properties:
enableMetrics:
default: true
description: EnableMetrics indicates the observability addon push
metrics to hub server.
type: boolean
interval:
default: 30
default: 300
description: Interval for the observability addon push metrics to
hub server.
format: int32
maximum: 3600
minimum: 15
type: integer
resources:
description: Resource requirement for metrics-collector
properties:
claims:
description: |-
Claims lists the names of resources, defined in spec.resourceClaims,
that are used by this container.
This is an alpha field and requires enabling the
DynamicResourceAllocation feature gate.
This field is immutable. It can only be set for containers.
items:
description: ResourceClaim references one entry in PodSpec.ResourceClaims.
properties:
name:
description: |-
Name must match the name of one entry in pod.spec.resourceClaims of
the Pod where this field is used. It makes that resource available
inside a container.
type: string
required:
- name
type: object
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
limits:
additionalProperties:
anyOf:
- type: integer
- type: string
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
description: |-
Limits describes the maximum amount of compute resources allowed.
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
type: object
requests:
additionalProperties:
anyOf:
- type: integer
- type: string
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
description: |-
Requests describes the minimum amount of compute resources required.
If Requests is omitted for a container, it defaults to Limits if that is explicitly specified,
otherwise to an implementation-defined value. Requests cannot exceed Limits.
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
type: object
type: object
scrapeSizeLimitBytes:
default: 1073741824
description: |-
ScrapeSizeLimitBytes is the max size in bytes for a single metrics scrape from in-cluster Prometheus.
Default is 1 GiB.
type: integer
workers:
default: 1
description: |-
Workers is the number of workers in metrics-collector that work in parallel to
push metrics to hub server. If set to > 1, metrics-collector will shard
/federate calls to Prometheus, based on matcher rules provided by allowlist.
Ensure that number of matchers exceeds number of workers.
format: int32
minimum: 1
type: integer
type: object
status:
description: ObservabilityAddonStatus defines the observed state of ObservabilityAddon
Expand Down Expand Up @@ -90,5 +165,5 @@ status:
acceptedNames:
kind: ""
plural: ""
conditions: []
storedVersions: []
conditions: null
storedVersions: null
50 changes: 36 additions & 14 deletions operators/multiclusterobservability/controllers/placementrule/manifestwork.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
workv1 "open-cluster-management.io/api/work/v1"

mcoshared "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/shared"
mcov1beta1 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta1"
mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2"
cert_controller "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/certificates"
Expand Down Expand Up @@ -940,6 +939,12 @@ func generateMetricsListCM(client client.Client) (*corev1.ConfigMap, *corev1.Con
return metricsAllowlistCM, ocp311AllowlistCM, nil
}

// getObservabilityAddon gets the ObservabilityAddon in the spoke namespace in the hub cluster.
// This is then synced to the actual spoke, by injecting it into the manifestwork.
// We assume that an existing addon will always be found here as we create it initially.
// If the addon is found with the mco source annotation, it will update the existing addon with the new values from MCO
// If the addon is found with the override source annotation, it will not update the existing addon but it will use the existing values.
// If the addon is found without any source annotation, it will add the mco source annotation and use the MCO values (upgrade case from ACM 2.12.2).
func getObservabilityAddon(c client.Client, namespace string,
mco *mcov1beta2.MultiClusterObservability) (*mcov1beta1.ObservabilityAddon, error) {
if namespace == config.GetDefaultNamespace() {
Expand All @@ -952,30 +957,47 @@ func getObservabilityAddon(c client.Client, namespace string,
}
err := c.Get(context.TODO(), namespacedName, found)
if err != nil {
if k8serrors.IsNotFound(err) {
return nil, nil
if !k8serrors.IsNotFound(err) {
log.Error(err, "Failed to check observabilityAddon")
return nil, err
}
log.Error(err, "Failed to check observabilityAddon")
return nil, err
}
if found.ObjectMeta.DeletionTimestamp != nil {
return nil, nil
}
return &mcov1beta1.ObservabilityAddon{

addon := &mcov1beta1.ObservabilityAddon{
TypeMeta: metav1.TypeMeta{
APIVersion: "observability.open-cluster-management.io/v1beta1",
Kind: "ObservabilityAddon",
},
ObjectMeta: metav1.ObjectMeta{
Name: obsAddonName,
Namespace: spokeNameSpace,
},
Spec: mcoshared.ObservabilityAddonSpec{
EnableMetrics: mco.Spec.ObservabilityAddonSpec.EnableMetrics,
Interval: mco.Spec.ObservabilityAddonSpec.Interval,
Resources: config.GetOBAResources(mco.Spec.ObservabilityAddonSpec, mco.Spec.InstanceSize),
Name: obsAddonName,
Namespace: spokeNameSpace,
Annotations: make(map[string]string),
},
}, nil
}

// Handle cases where the addon doesn't have the annotation
if found.Annotations == nil {
found.Annotations = make(map[string]string)
}

if _, ok := found.Annotations[addonSourceAnnotation]; !ok {
found.Annotations[addonSourceAnnotation] = addonSourceMCO
}

addon.Annotations = found.Annotations

if found.Annotations[addonSourceAnnotation] == addonSourceMCO {
setObservabilityAddonSpec(addon, mco.Spec.ObservabilityAddonSpec, config.GetOBAResources(mco.Spec.ObservabilityAddonSpec, mco.Spec.InstanceSize))
}

if found.Annotations[addonSourceAnnotation] == addonSourceOverride {
setObservabilityAddonSpec(addon, &found.Spec, found.Spec.Resources)
}

return addon, nil
}

func removeObservabilityAddon(client client.Client, namespace string) error {
Expand Down
2 changes: 1 addition & 1 deletion operators/multiclusterobservability/controllers/placementrule/manifestwork_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (

const (
pullSecretName = "test-pull-secret"
workSize = 13
workSize = 14
)

func init() {
Expand Down
51 changes: 48 additions & 3 deletions operators/multiclusterobservability/controllers/placementrule/obsaddon.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,31 @@ package placementrule

import (
"context"
"fmt"
"time"

"github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config"

"golang.org/x/exp/slices"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

obshared "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/shared"
obsv1beta1 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta1"
mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2"
"github.com/stolostron/multicluster-observability-operator/operators/pkg/util"
)

const (
obsAddonName = "observability-addon"
obsAddonFinalizer = "observability.open-cluster-management.io/addon-cleanup"
obsAddonName = "observability-addon"
obsAddonFinalizer = "observability.open-cluster-management.io/addon-cleanup"
addonSourceAnnotation = "observability.open-cluster-management.io/addon-source"
addonSourceMCO = "mco"
addonSourceOverride = "override"
)

func deleteObsAddon(c client.Client, namespace string) error {
Expand Down Expand Up @@ -58,7 +66,11 @@ func deleteObsAddon(c client.Client, namespace string) error {
return nil
}

func createObsAddon(c client.Client, namespace string) error {
// createObsAddon creates the default ObservabilityAddon in the spoke namespace in the hub cluster.
// It will initially mirror values from the MultiClusterObservability CR with the mco source annotation.
// If an existing addon is found with the mco source annotation it will update the existing addon with the new values.
// If the existing addon is created by the user with the override source annotation, it will not update the existing addon.
func createObsAddon(mco *mcov1beta2.MultiClusterObservability, c client.Client, namespace string) error {
if namespace == config.GetDefaultNamespace() {
return nil
}
Expand All @@ -70,11 +82,19 @@ func createObsAddon(c client.Client, namespace string) error {
ObjectMeta: metav1.ObjectMeta{
Name: obsAddonName,
Namespace: namespace,
Annotations: map[string]string{
addonSourceAnnotation: addonSourceMCO,
},
Labels: map[string]string{
ownerLabelKey: ownerLabelValue,
},
},
}

if mco.Spec.ObservabilityAddonSpec != nil {
setObservabilityAddonSpec(ec, mco.Spec.ObservabilityAddonSpec, config.GetOBAResources(mco.Spec.ObservabilityAddonSpec, mco.Spec.InstanceSize))
}

found := &obsv1beta1.ObservabilityAddon{}
err := c.Get(context.TODO(), types.NamespacedName{Name: obsAddonName, Namespace: namespace}, found)
if err != nil && errors.IsNotFound(err) || err == nil && found.GetDeletionTimestamp() != nil {
Expand All @@ -96,6 +116,20 @@ func createObsAddon(c client.Client, namespace string) error {
return err
}

// Check if existing addon was created by MCO
if found.Annotations != nil && found.Annotations[addonSourceAnnotation] == addonSourceMCO {
// Only update if specs are different
if !equality.Semantic.DeepEqual(found.Spec, ec.Spec) {
found.Spec = ec.Spec
err = c.Update(context.TODO(), found)
if err != nil {
return fmt.Errorf("failed to update observabilityaddon cr: %w", err)
}
log.Info("observabilityaddon updated", "namespace", namespace)
return nil
}
}

log.Info("observabilityaddon already existed/unchanged", "namespace", namespace)
return nil
}
Expand Down Expand Up @@ -145,3 +179,14 @@ func deleteFinalizer(c client.Client, obsaddon *obsv1beta1.ObservabilityAddon) e
}
return nil
}

// setObservabilityAddonSpec sets the ObservabilityAddon spec fields from the given MCO spec
func setObservabilityAddonSpec(addonSpec *obsv1beta1.ObservabilityAddon, desiredSpec *obshared.ObservabilityAddonSpec, resources *corev1.ResourceRequirements) {
if desiredSpec != nil {
addonSpec.Spec.EnableMetrics = desiredSpec.EnableMetrics
addonSpec.Spec.Interval = desiredSpec.Interval
addonSpec.Spec.ScrapeSizeLimitBytes = desiredSpec.ScrapeSizeLimitBytes
addonSpec.Spec.Workers = desiredSpec.Workers
addonSpec.Spec.Resources = resources
}
}
Loading

0 comments on commit 7db60ca

Please sign in to comment.