Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Open up util functions for context propagation #1033

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 1 addition & 1 deletion components/kserve/kserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
}

// For odh-model-controller
if err := cluster.UpdatePodSecurityRolebinding(cli, dscispec.ApplicationsNamespace, "odh-model-controller"); err != nil {
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/modelregistry/modelregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (m *ModelRegistry) GetComponentName() string {
return ComponentName
}

func (m *ModelRegistry) ReconcileComponent(_ context.Context, cli client.Client, logger logr.Logger,
func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error {
l := m.ConfigComponentLogger(logger, ComponentName, dscispec)
var imageParamMap = map[string]string{
Expand Down Expand Up @@ -90,7 +90,7 @@ func (m *ModelRegistry) ReconcileComponent(_ context.Context, cli client.Client,

// Create odh-model-registries namespace
// We do not delete this namespace even when ModelRegistry is Removed or when operator is uninstalled.
_, err := cluster.CreateNamespace(cli, "odh-model-registries")
_, err := cluster.CreateNamespace(ctx, cli, "odh-model-registries")
if 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 @@ -77,7 +77,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 @@ -55,7 +55,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 @@ -69,10 +69,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 @@ -87,13 +87,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 @@ -102,7 +102,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 @@ -118,7 +118,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 @@ -128,7 +128,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 @@ -140,11 +140,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 @@ -55,6 +57,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
Loading