Skip to content

Commit

Permalink
Cleanup kustomize manifests rendering action (#1353)
Browse files Browse the repository at this point in the history
This commit is meant to cleanup the kustomize manifest redering by:
- removing a useless parameter related to the caching mechanism
- moving the code to the kustomize package to accomodate other rendering
  engines
- introduce metrics tro track how many resources have been rendered and
  to perform blackbox testing against the action
  • Loading branch information
lburgazzoli authored Nov 8, 2024
1 parent 4906ffe commit f941791
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 64 deletions.
10 changes: 5 additions & 5 deletions controllers/components/dashboard/dashboard_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (

componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/security"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources"
Expand Down Expand Up @@ -74,8 +74,8 @@ func NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
WithAction(devFlags).
WithAction(configureDependencies).
WithAction(security.NewUpdatePodSecurityRoleBindingAction(serviceAccounts)).
WithAction(render.NewAction(
render.WithCache(true, render.DefaultCachingKeyFn),
WithAction(kustomize.NewAction(
kustomize.WithCache(kustomize.DefaultCachingKeyFn),
// Those are the default labels added by the legacy deploy method
// and should be preserved as the original plugin were affecting
// deployment selectors that are immutable once created, so it won't
Expand All @@ -84,8 +84,8 @@ func NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
//
// Additional labels/annotations MUST be added by the deploy action
// so they would affect only objects metadata without side effects
render.WithLabel(labels.ODH.Component(componentName), "true"),
render.WithLabel(labels.K8SCommon.PartOf, componentName),
kustomize.WithLabel(labels.ODH.Component(componentName), "true"),
kustomize.WithLabel(labels.K8SCommon.PartOf, componentName),
)).
WithAction(customizeResources).
WithAction(deploy.NewAction(
Expand Down
10 changes: 5 additions & 5 deletions controllers/components/modelregistry/modelregistry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (

componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources"
Expand Down Expand Up @@ -74,10 +74,10 @@ func NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
WithAction(checkPreConditions).
WithAction(initialize).
WithAction(configureDependencies).
WithAction(render.NewAction(
render.WithCache(true, render.DefaultCachingKeyFn),
render.WithLabel(labels.ODH.Component(ComponentName), "true"),
render.WithLabel(labels.K8SCommon.PartOf, ComponentName),
WithAction(kustomize.NewAction(
kustomize.WithCache(kustomize.DefaultCachingKeyFn),
kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"),
kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName),
)).
WithAction(customizeResources).
WithAction(deploy.NewAction(
Expand Down
10 changes: 5 additions & 5 deletions controllers/components/ray/ray_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (

componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/reconciler"
Expand All @@ -56,10 +56,10 @@ func NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
// Add Ray-specific actions
WithAction(initialize).
WithAction(devFlags).
WithAction(render.NewAction(
render.WithCache(true, render.DefaultCachingKeyFn),
render.WithLabel(labels.ODH.Component(ComponentName), "true"),
render.WithLabel(labels.K8SCommon.PartOf, ComponentName),
WithAction(kustomize.NewAction(
kustomize.WithCache(kustomize.DefaultCachingKeyFn),
kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"),
kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName),
)).
WithAction(deploy.NewAction(
deploy.WithFieldOwner(componentsv1.RayInstanceName),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
package render
package kustomize

import (
"bytes"
"context"
"encoding/base64"
"fmt"
"strings"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/kustomize/kyaml/filesys"

"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/manifests/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
)

const RendererEngine = "kustomize"

type CachingKeyFn func(_ context.Context, rr *types.ReconciliationRequest) ([]byte, error)

// Action takes a set of manifest locations and render them as Unstructured resources for
Expand All @@ -25,7 +27,6 @@ type Action struct {
keOpts []kustomize.EngineOptsFn
ke *kustomize.Engine

cacheAnnotation bool
cachingKeyFn CachingKeyFn
cachingKey []byte
cachedResources resources.UnstructuredList
Expand Down Expand Up @@ -69,9 +70,8 @@ func WithManifestsOptions(values ...kustomize.EngineOptsFn) ActionOpts {
}
}

func WithCache(addHashAnnotation bool, value CachingKeyFn) ActionOpts {
func WithCache(value CachingKeyFn) ActionOpts {
return func(action *Action) {
action.cacheAnnotation = addHashAnnotation
action.cachingKeyFn = value
}
}
Expand Down Expand Up @@ -101,20 +101,15 @@ func (a *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error
return fmt.Errorf("unable to render reconciliation object: %w", err)
}

// Add a letter at the beginning and use URL safe encoding
digest := "v" + base64.RawURLEncoding.EncodeToString(cachingKey)
result = res

if len(cachingKey) != 0 {
if a.cacheAnnotation {
for i := range result {
resources.SetAnnotation(&result[i], annotations.ComponentHash, digest)
}
}

a.cachingKey = cachingKey
a.cachedResources = result
}

controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind)
render.RenderedResourcesTotal.WithLabelValues(controllerName, RendererEngine).Add(float64(len(result)))
}

// deep copy object so changes done in the pipelines won't
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package render
package kustomize

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package render_test
package kustomize_test

import (
"context"
"path"
"testing"

"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/rs/xid"
"sigs.k8s.io/kustomize/kyaml/filesys"

Expand All @@ -13,9 +14,9 @@ import (
dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/manifests/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
mk "github.com/opendatahub-io/opendatahub-operator/v2/pkg/manifests/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq"
Expand Down Expand Up @@ -104,8 +105,8 @@ func TestRenderResourcesAction(t *testing.T) {
id := xid.New().String()
fs := filesys.MakeFsInMemory()

_ = fs.MkdirAll(path.Join(id, kustomize.DefaultKustomizationFilePath))
_ = fs.WriteFile(path.Join(id, kustomize.DefaultKustomizationFileName), []byte(testRenderResourcesKustomization))
_ = fs.MkdirAll(path.Join(id, mk.DefaultKustomizationFilePath))
_ = fs.WriteFile(path.Join(id, mk.DefaultKustomizationFileName), []byte(testRenderResourcesKustomization))
_ = fs.WriteFile(path.Join(id, "test-resources-cm.yaml"), []byte(testRenderResourcesConfigMap))
_ = fs.WriteFile(path.Join(id, "test-resources-deployment-managed.yaml"), []byte(testRenderResourcesManaged))
_ = fs.WriteFile(path.Join(id, "test-resources-deployment-unmanaged.yaml"), []byte(testRenderResourcesUnmanaged))
Expand All @@ -114,14 +115,14 @@ func TestRenderResourcesAction(t *testing.T) {
cl, err := fakeclient.New(ctx)
g.Expect(err).ShouldNot(HaveOccurred())

action := render.NewAction(
render.WithLabel("component.opendatahub.io/name", "foo"),
render.WithLabel("platform.opendatahub.io/namespace", ns),
render.WithAnnotation("platform.opendatahub.io/release", "1.2.3"),
render.WithAnnotation("platform.opendatahub.io/type", "managed"),
action := kustomize.NewAction(
kustomize.WithLabel("component.opendatahub.io/name", "foo"),
kustomize.WithLabel("platform.opendatahub.io/namespace", ns),
kustomize.WithAnnotation("platform.opendatahub.io/release", "1.2.3"),
kustomize.WithAnnotation("platform.opendatahub.io/type", "managed"),
// for testing
render.WithManifestsOptions(
kustomize.WithEngineFS(fs),
kustomize.WithManifestsOptions(
mk.WithEngineFS(fs),
),
)

Expand Down Expand Up @@ -184,32 +185,32 @@ func TestRenderResourcesWithCacheAction(t *testing.T) {
id := xid.New().String()
fs := filesys.MakeFsInMemory()

_ = fs.MkdirAll(path.Join(id, kustomize.DefaultKustomizationFilePath))
_ = fs.WriteFile(path.Join(id, kustomize.DefaultKustomizationFileName), []byte(testRenderResourcesWithCacheKustomization))
_ = fs.MkdirAll(path.Join(id, mk.DefaultKustomizationFilePath))
_ = fs.WriteFile(path.Join(id, mk.DefaultKustomizationFileName), []byte(testRenderResourcesWithCacheKustomization))
_ = fs.WriteFile(path.Join(id, "test-resources-deployment.yaml"), []byte(testRenderResourcesWithCacheDeployment))

cl, err := fakeclient.New(ctx)
g.Expect(err).ShouldNot(HaveOccurred())

action := render.NewAction(
render.WithCache(true, render.DefaultCachingKeyFn),
render.WithLabel(labels.ComponentPartOf, "foo"),
render.WithLabel("platform.opendatahub.io/namespace", ns),
render.WithAnnotation("platform.opendatahub.io/release", "1.2.3"),
render.WithAnnotation("platform.opendatahub.io/type", "managed"),
action := kustomize.NewAction(
kustomize.WithCache(kustomize.DefaultCachingKeyFn),
kustomize.WithLabel(labels.ComponentPartOf, "foo"),
kustomize.WithLabel("platform.opendatahub.io/namespace", ns),
kustomize.WithAnnotation("platform.opendatahub.io/release", "1.2.3"),
kustomize.WithAnnotation("platform.opendatahub.io/type", "managed"),
// for testing
render.WithManifestsOptions(
kustomize.WithEngineFS(fs),
kustomize.WithManifestsOptions(
mk.WithEngineFS(fs),
),
)

hash := ""
render.RenderedResourcesTotal.Reset()

for i := int64(0); i < 3; i++ {
d := componentsv1.Dashboard{}

if i == 2 {
d.Generation = i
if i >= 1 {
d.Generation = 1
}

rr := types.ReconciliationRequest{
Expand All @@ -235,14 +236,15 @@ func TestRenderResourcesWithCacheAction(t *testing.T) {
)),
))

newHash := rr.Resources[0].GetAnnotations()[annotations.ComponentHash]
if i == 1 {
g.Expect(newHash).Should(Equal(hash))
}
if i == 2 {
g.Expect(newHash).ShouldNot(Equal(hash))
}
rc := testutil.ToFloat64(render.RenderedResourcesTotal)

hash = newHash
switch i {
case 0:
g.Expect(rc).Should(BeNumerically("==", 1))
case 1:
g.Expect(rc).Should(BeNumerically("==", 2))
case 2:
g.Expect(rc).Should(BeNumerically("==", 2))
}
}
}
32 changes: 32 additions & 0 deletions pkg/controller/actions/render/render_metrics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package render

import (
"github.com/prometheus/client_golang/prometheus"
"sigs.k8s.io/controller-runtime/pkg/metrics"
)

var (
// RenderedResourcesTotal is a prometheus counter metrics which holds the total
// number of resource rendered by the action per controller and rendering type.
// It has two labels.
// controller label refers to the controller name.
// engine label refers to the rendering engine.
RenderedResourcesTotal = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "action_renderer_manifests_total",
Help: "Number of rendered resources",
},
[]string{
"controller",
"engine",
},
)
)

// init register metrics to the global registry from controller-runtime/pkg/metrics.
// see https://book.kubebuilder.io/reference/metrics#publishing-additional-metrics
//
//nolint:gochecknoinits
func init() {
metrics.Registry.MustRegister(RenderedResourcesTotal)
}
1 change: 0 additions & 1 deletion pkg/metadata/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const ManagementStateAnnotation = "component.opendatahub.io/management-state"

const (
ComponentGeneration = "components.opendatahub.io/generation"
ComponentHash = "components.opendatahub.io/hash"
PlatformVersion = "platform.opendatahub.io/version"
PlatformType = "platform.opendatahub.io/type"
)

0 comments on commit f941791

Please sign in to comment.