Skip to content

Commit

Permalink
chore: Open up util functions for context propagation (opendatahub-io…
Browse files Browse the repository at this point in the history
…#1033)

context should be determined by the caller and propagated
down the call chain.
  • Loading branch information
aslakknutsen authored and zdtsw committed Jun 18, 2024
1 parent 1269d52 commit 48b15b9
Show file tree
Hide file tree
Showing 12 changed files with 40 additions and 29 deletions.
6 changes: 3 additions & 3 deletions components/dashboard/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,14 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,

// 2. platform specific RBAC
if platform == cluster.OpenDataHub || platform == "" {
err := cluster.UpdatePodSecurityRolebinding(cli, dscispec.ApplicationsNamespace, "odh-dashboard")
err := cluster.UpdatePodSecurityRolebinding(ctx, cli, dscispec.ApplicationsNamespace, "odh-dashboard")
if err != nil {
return err
}
}

if platform == cluster.SelfManagedRhods || platform == cluster.ManagedRhods {
err := cluster.UpdatePodSecurityRolebinding(cli, dscispec.ApplicationsNamespace, "rhods-dashboard")
err := cluster.UpdatePodSecurityRolebinding(ctx, cli, dscispec.ApplicationsNamespace, "rhods-dashboard")
if err != nil {
return err
}
Expand All @@ -152,7 +152,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
switch platform {
case cluster.SelfManagedRhods, cluster.ManagedRhods:
// anaconda
if err := cluster.CreateSecret(cli, "anaconda-ce-access", dscispec.ApplicationsNamespace); err != nil {
if err := cluster.CreateSecret(ctx, cli, "anaconda-ce-access", dscispec.ApplicationsNamespace); err != nil {
return fmt.Errorf("failed to create access-secret for anaconda: %w", err)
}
// overlay which including ../../base + anaconda-ce-validator
Expand Down
4 changes: 3 additions & 1 deletion components/kserve/kserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,9 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
l.WithValues("Path", Path).Info("apply manifests done for kserve")
// For odh-model-controller
if enabled {
if err := cluster.UpdatePodSecurityRolebinding(cli, dscispec.ApplicationsNamespace, "odh-model-controller"); err != nil {

// For odh-model-controller
if err := cluster.UpdatePodSecurityRolebinding(ctx, cli, dscispec.ApplicationsNamespace, "odh-model-controller"); err != nil {
return err
}
// Update image parameters for odh-model-controller
Expand Down
4 changes: 2 additions & 2 deletions components/modelmeshserving/modelmeshserving.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
}
}

if err := cluster.UpdatePodSecurityRolebinding(cli, dscispec.ApplicationsNamespace,
if err := cluster.UpdatePodSecurityRolebinding(ctx, cli, dscispec.ApplicationsNamespace,
"modelmesh",
"modelmesh-controller",
"odh-prometheus-operator",
Expand All @@ -129,7 +129,7 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
l.WithValues("Path", Path).Info("apply manifests done for modelmesh")
// For odh-model-controller
if enabled {
if err := cluster.UpdatePodSecurityRolebinding(cli, dscispec.ApplicationsNamespace,
if err := cluster.UpdatePodSecurityRolebinding(ctx, cli, dscispec.ApplicationsNamespace,
"odh-model-controller"); err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions components/workbenches/workbenches.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,13 @@ func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client,
if platform == cluster.SelfManagedRhods || platform == cluster.ManagedRhods {
// Intentionally leaving the ownership unset for this namespace.
// Specifying this label triggers its deletion when the operator is uninstalled.
_, err := cluster.CreateNamespace(cli, "rhods-notebooks", cluster.WithLabels(labels.ODH.OwnedNamespace, "true"))
_, err := cluster.CreateNamespace(ctx, cli, "rhods-notebooks", cluster.WithLabels(labels.ODH.OwnedNamespace, "true"))
if err != nil {
return err
}
}
// Update Default rolebinding
err = cluster.UpdatePodSecurityRolebinding(cli, dscispec.ApplicationsNamespace, "notebook-controller-service-account")
err = cluster.UpdatePodSecurityRolebinding(ctx, cli, dscispec.ApplicationsNamespace, "notebook-controller-service-account")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/dscinitialization/monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (r *DSCInitializationReconciler) configureManagedMonitoring(ctx context.Con
}

if initial == "init" {
err := cluster.UpdatePodSecurityRolebinding(r.Client, dscInit.Spec.Monitoring.Namespace, "redhat-ods-monitoring")
err := cluster.UpdatePodSecurityRolebinding(ctx, r.Client, dscInit.Spec.Monitoring.Namespace, "redhat-ods-monitoring")
if err != nil {
return fmt.Errorf("error to update monitoring security rolebinding: %w", err)
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/cluster/cluster_operations_int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var _ = Describe("Creating cluster resources", func() {
defer objectCleaner.DeleteAll(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}})

// when
ns, err := cluster.CreateNamespace(envTestClient, namespace)
ns, err := cluster.CreateNamespace(context.Background(), envTestClient, namespace)

// then
Expect(err).ToNot(HaveOccurred())
Expand All @@ -57,7 +57,7 @@ var _ = Describe("Creating cluster resources", func() {
defer objectCleaner.DeleteAll(newNamespace)

// when
existingNamespace, err := cluster.CreateNamespace(envTestClient, namespace)
existingNamespace, err := cluster.CreateNamespace(context.Background(), envTestClient, namespace)

// then
Expect(err).ToNot(HaveOccurred())
Expand All @@ -70,7 +70,7 @@ var _ = Describe("Creating cluster resources", func() {
defer objectCleaner.DeleteAll(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}})

// when
nsWithLabels, err := cluster.CreateNamespace(envTestClient, namespace, cluster.WithLabels("opendatahub.io/test-label", "true"))
nsWithLabels, err := cluster.CreateNamespace(context.Background(), envTestClient, namespace, cluster.WithLabels("opendatahub.io/test-label", "true"))

// then
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -103,6 +103,7 @@ var _ = Describe("Creating cluster resources", func() {

// when
err := cluster.CreateOrUpdateConfigMap(
context.Background(),
envTestClient,
configMap,
cluster.WithLabels(labels.K8SCommon.PartOf, "opendatahub"),
Expand All @@ -129,6 +130,7 @@ var _ = Describe("Creating cluster resources", func() {
It("should be able to update existing config map", func() {
// given
createErr := cluster.CreateOrUpdateConfigMap(
context.Background(),
envTestClient,
&v1.ConfigMap{
ObjectMeta: configMapMeta,
Expand All @@ -150,6 +152,7 @@ var _ = Describe("Creating cluster resources", func() {
}

updateErr := cluster.CreateOrUpdateConfigMap(
context.Background(),
envTestClient,
updatedConfigMap,
cluster.WithLabels("test-step", "update-existing-configmap"),
Expand Down
26 changes: 13 additions & 13 deletions pkg/cluster/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import (

// UpdatePodSecurityRolebinding update default rolebinding which is created in applications namespace by manifests
// being used by different components and SRE monitoring.
func UpdatePodSecurityRolebinding(cli client.Client, namespace string, serviceAccountsList ...string) error {
func UpdatePodSecurityRolebinding(ctx context.Context, cli client.Client, namespace string, serviceAccountsList ...string) error {
foundRoleBinding := &authv1.RoleBinding{}
if err := cli.Get(context.TODO(), client.ObjectKey{Name: namespace, Namespace: namespace}, foundRoleBinding); err != nil {
if err := cli.Get(ctx, client.ObjectKey{Name: namespace, Namespace: namespace}, foundRoleBinding); err != nil {
return fmt.Errorf("error to get rolebinding %s from namespace %s: %w", namespace, namespace, err)
}

Expand All @@ -35,7 +35,7 @@ func UpdatePodSecurityRolebinding(cli client.Client, namespace string, serviceAc
}
}

if err := cli.Update(context.TODO(), foundRoleBinding); err != nil {
if err := cli.Update(ctx, foundRoleBinding); err != nil {
return fmt.Errorf("error update rolebinding %s with serviceaccount: %w", namespace, err)
}

Expand All @@ -54,7 +54,7 @@ func subjectExistInRoleBinding(subjectList []authv1.Subject, serviceAccountName,
}

// CreateSecret creates secrets required by dashboard component in downstream.
func CreateSecret(cli client.Client, name, namespace string, metaOptions ...MetaOptions) error {
func CreateSecret(ctx context.Context, cli client.Client, name, namespace string, metaOptions ...MetaOptions) error {
desiredSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand All @@ -68,10 +68,10 @@ func CreateSecret(cli client.Client, name, namespace string, metaOptions ...Meta
}

foundSecret := &corev1.Secret{}
err := cli.Get(context.TODO(), client.ObjectKey{Name: name, Namespace: namespace}, foundSecret)
err := cli.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, foundSecret)
if err != nil {
if apierrs.IsNotFound(err) {
err = cli.Create(context.TODO(), desiredSecret)
err = cli.Create(ctx, desiredSecret)
if err != nil && !apierrs.IsAlreadyExists(err) {
return err
}
Expand All @@ -85,13 +85,13 @@ func CreateSecret(cli client.Client, name, namespace string, metaOptions ...Meta
// CreateOrUpdateConfigMap creates a new configmap or updates an existing one.
// If the configmap already exists, it will be updated with the merged Data and MetaOptions, if any.
// ConfigMap.ObjectMeta.Name and ConfigMap.ObjectMeta.Namespace are both required, it returns an error otherwise.
func CreateOrUpdateConfigMap(c client.Client, desiredCfgMap *corev1.ConfigMap, metaOptions ...MetaOptions) error {
func CreateOrUpdateConfigMap(ctx context.Context, c client.Client, desiredCfgMap *corev1.ConfigMap, metaOptions ...MetaOptions) error {
if desiredCfgMap.GetName() == "" || desiredCfgMap.GetNamespace() == "" {
return fmt.Errorf("configmap name and namespace must be set")
}

existingCfgMap := &corev1.ConfigMap{}
err := c.Get(context.TODO(), client.ObjectKey{
err := c.Get(ctx, client.ObjectKey{
Name: desiredCfgMap.Name,
Namespace: desiredCfgMap.Namespace,
}, existingCfgMap)
Expand All @@ -100,7 +100,7 @@ func CreateOrUpdateConfigMap(c client.Client, desiredCfgMap *corev1.ConfigMap, m
if applyErr := ApplyMetaOptions(desiredCfgMap, metaOptions...); applyErr != nil {
return applyErr
}
return c.Create(context.TODO(), desiredCfgMap)
return c.Create(ctx, desiredCfgMap)
} else if err != nil {
return err
}
Expand All @@ -116,7 +116,7 @@ func CreateOrUpdateConfigMap(c client.Client, desiredCfgMap *corev1.ConfigMap, m
existingCfgMap.Data[key] = value
}

if updateErr := c.Update(context.TODO(), existingCfgMap); updateErr != nil {
if updateErr := c.Update(ctx, existingCfgMap); updateErr != nil {
return updateErr
}

Expand All @@ -126,7 +126,7 @@ func CreateOrUpdateConfigMap(c client.Client, desiredCfgMap *corev1.ConfigMap, m

// CreateNamespace creates a namespace and apply metadata.
// If a namespace already exists, the operation has no effect on it.
func CreateNamespace(cli client.Client, namespace string, metaOptions ...MetaOptions) (*corev1.Namespace, error) {
func CreateNamespace(ctx context.Context, cli client.Client, namespace string, metaOptions ...MetaOptions) (*corev1.Namespace, error) {
desiredNamespace := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: namespace,
Expand All @@ -138,11 +138,11 @@ func CreateNamespace(cli client.Client, namespace string, metaOptions ...MetaOpt
}

foundNamespace := &corev1.Namespace{}
if getErr := cli.Get(context.TODO(), client.ObjectKey{Name: namespace}, foundNamespace); client.IgnoreNotFound(getErr) != nil {
if getErr := cli.Get(ctx, client.ObjectKey{Name: namespace}, foundNamespace); client.IgnoreNotFound(getErr) != nil {
return nil, getErr
}

createErr := cli.Create(context.TODO(), desiredNamespace)
createErr := cli.Create(ctx, desiredNamespace)
if apierrs.IsAlreadyExists(createErr) {
return foundNamespace, nil
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/feature/resources.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
package feature

import (
"context"

"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
)

// CreateNamespaceIfNotExists will create a namespace with the given name if it does not exist yet.
// It does not set ownership nor apply extra metadata to the existing namespace.
func CreateNamespaceIfNotExists(namespace string) Action {
return func(f *Feature) error {
_, err := cluster.CreateNamespace(f.Client, namespace)
_, err := cluster.CreateNamespace(context.TODO(), f.Client, namespace)

return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/feature/servicemesh/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func EnsureAuthNamespaceExists(f *feature.Feature) error {
return resolveNsErr
}

_, err := cluster.CreateNamespace(f.Client, f.Spec.Auth.Namespace, feature.OwnedBy(f))
_, err := cluster.CreateNamespace(context.TODO(), f.Client, f.Spec.Auth.Namespace, feature.OwnedBy(f))
return err
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/feature/servicemesh/resources.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package servicemesh

import (
"context"
"strings"

corev1 "k8s.io/api/core/v1"
Expand All @@ -22,6 +23,7 @@ func MeshRefs(f *feature.Feature) error {
}

return cluster.CreateOrUpdateConfigMap(
context.TODO(),
f.Client,
&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -50,6 +52,7 @@ func AuthRefs(f *feature.Feature) error {
}

return cluster.CreateOrUpdateConfigMap(
context.TODO(),
f.Client,
&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/features/manifests_int_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package features_test

import (
"context"
"os"
"path"

Expand Down Expand Up @@ -29,7 +30,7 @@ var _ = Describe("Manifest sources", func() {
nsName := envtestutil.AppendRandomNameTo("smcp-ns")

var err error
namespace, err = cluster.CreateNamespace(envTestClient, nsName)
namespace, err = cluster.CreateNamespace(context.Background(), envTestClient, nsName)
Expect(err).ToNot(HaveOccurred())

dsci = fixtures.NewDSCInitialization(nsName)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/features/resources_int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var _ = Describe("Applying and updating resources", func() {
dummyAnnotation = "fake-anno"

var err error
namespace, err = cluster.CreateNamespace(envTestClient, testNamespace)
namespace, err = cluster.CreateNamespace(context.Background(), envTestClient, testNamespace)
Expect(err).ToNot(HaveOccurred())

dsci = fixtures.NewDSCInitialization(testNamespace)
Expand Down

0 comments on commit 48b15b9

Please sign in to comment.