Skip to content

Commit

Permalink
logger: blindly convert ctrl.Log users except webhook to contextual
Browse files Browse the repository at this point in the history 
All the users should have proper context. Webhook is a separate
server so makes own logger based on the controller-runtime's one.

The log level changes will affect it as well.

Signed-off-by: Yauheni Kaliuta <[email protected]>
  • Loading branch information
@ykaliuta
ykaliuta committed Oct 9, 2024
1 parent ad30b9b commit e2ed376
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 28 deletions.
5 changes: 3 additions & 2 deletions components/kserve/servicemesh_setup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (

operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
Expand Down Expand Up @@ -37,6 +37,7 @@ func (k *Kserve) removeServiceMeshConfigurations(ctx context.Context, cli client
}

func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec) feature.FeaturesProvider {
log := logf.FromContext(ctx)
return func(registry feature.FeaturesRegistry) error {
authorinoInstalled, err := cluster.SubscriptionExists(ctx, cli, "authorino-operator")
if err != nil {
Expand Down Expand Up @@ -68,7 +69,7 @@ func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Clien
return kserveExtAuthzErr
}
} else {
ctrl.Log.Info("WARN: Authorino operator is not installed on the cluster, skipping authorization capability")
log.Info("WARN: Authorino operator is not installed on the cluster, skipping authorization capability")
}

return nil
Expand Down
8 changes: 5 additions & 3 deletions pkg/cluster/resources.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
)
Expand Down Expand Up @@ -182,6 +182,7 @@ func ExecuteOnAllNamespaces(ctx context.Context, cli client.Client, processFunc

// WaitForDeploymentAvailable to check if component deployment from 'namespace' is ready within 'timeout' before apply prometheus rules for the component.
func WaitForDeploymentAvailable(ctx context.Context, c client.Client, componentName string, namespace string, interval int, timeout int) error {
log := logf.FromContext(ctx)
resourceInterval := time.Duration(interval) * time.Second
resourceTimeout := time.Duration(timeout) * time.Minute

Expand All @@ -192,7 +193,7 @@ func WaitForDeploymentAvailable(ctx context.Context, c client.Client, componentN
return false, fmt.Errorf("error fetching list of deployments: %w", err)
}

ctrl.Log.Info("waiting for " + strconv.Itoa(len(componentDeploymentList.Items)) + " deployment to be ready for " + componentName)
log.Info("waiting for " + strconv.Itoa(len(componentDeploymentList.Items)) + " deployment to be ready for " + componentName)
for _, deployment := range componentDeploymentList.Items {
if deployment.Status.ReadyReplicas != deployment.Status.Replicas {
return false, nil
Expand All @@ -204,6 +205,7 @@ func WaitForDeploymentAvailable(ctx context.Context, c client.Client, componentN
}

func CreateWithRetry(ctx context.Context, cli client.Client, obj client.Object, timeoutMin int) error {
log := logf.FromContext(ctx)
interval := time.Second * 5 // arbitrary value
timeout := time.Duration(timeoutMin) * time.Minute

Expand Down Expand Up @@ -231,7 +233,7 @@ func CreateWithRetry(ctx context.Context, cli client.Client, obj client.Object,

// retry if 500, assume webhook is not available
if k8serr.IsInternalError(errCreate) {
ctrl.Log.Info("Error creating object, retrying...", "reason", errCreate)
log.Info("Error creating object, retrying...", "reason", errCreate)
return false, nil
}

Expand Down
15 changes: 9 additions & 6 deletions pkg/upgrade/uninstallation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
k8serr "k8s.io/apimachinery/pkg/api/errors"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
Expand All @@ -25,6 +26,7 @@ const (
// OperatorUninstall deletes all the externally generated resources.
// This includes DSCI, namespace created by operator (but not workbench or MR's), subscription and CSV.
func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster.Platform) error {
log := logf.FromContext(ctx)
if err := removeDSCInitialization(ctx, cli); err != nil {
return err
}
Expand All @@ -51,7 +53,7 @@ func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster.
if err := cli.Delete(ctx, &namespace); err != nil {
return fmt.Errorf("error deleting namespace %v: %w", namespace.Name, err)
}
ctrl.Log.Info("Namespace " + namespace.Name + " deleted as a part of uninstallation.")
log.Info("Namespace " + namespace.Name + " deleted as a part of uninstallation.")
}
}

Expand All @@ -66,7 +68,7 @@ func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster.
return err
}

ctrl.Log.Info("Removing operator subscription which in turn will remove installplan")
log.Info("Removing operator subscription which in turn will remove installplan")
subsName := "opendatahub-operator"
if platform == cluster.SelfManagedRhods {
subsName = "rhods-operator"
Expand All @@ -77,10 +79,10 @@ func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster.
}
}

ctrl.Log.Info("Removing the operator CSV in turn remove operator deployment")
log.Info("Removing the operator CSV in turn remove operator deployment")
err = removeCSV(ctx, cli)

ctrl.Log.Info("All resources deleted as part of uninstall.")
log.Info("All resources deleted as part of uninstall.")
return err
}

Expand Down Expand Up @@ -126,6 +128,7 @@ func HasDeleteConfigMap(ctx context.Context, c client.Client) bool {
}

func removeCSV(ctx context.Context, c client.Client) error {
log := logf.FromContext(ctx)
// Get watchNamespace
operatorNamespace, err := cluster.GetOperatorNamespace()
if err != nil {
Expand All @@ -142,7 +145,7 @@ func removeCSV(ctx context.Context, c client.Client) error {
return err
}

ctrl.Log.Info("Deleting CSV " + operatorCsv.Name)
log.Info("Deleting CSV " + operatorCsv.Name)
err = c.Delete(ctx, operatorCsv)
if err != nil {
if k8serr.IsNotFound(err) {
Expand All @@ -151,7 +154,7 @@ func removeCSV(ctx context.Context, c client.Client) error {

return fmt.Errorf("error deleting clusterserviceversion: %w", err)
}
ctrl.Log.Info("Clusterserviceversion " + operatorCsv.Name + " deleted as a part of uninstall")
log.Info("Clusterserviceversion " + operatorCsv.Name + " deleted as a part of uninstall")

return nil
}
41 changes: 24 additions & 17 deletions pkg/upgrade/upgrade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
Expand Down Expand Up @@ -115,6 +115,7 @@ func CreateDefaultDSC(ctx context.Context, cli client.Client) error {
// If there exists default-dsci instance already, it will not update DSCISpec on it.
// Note: DSCI CR modifcations are not supported, as it is the initial prereq setting for the components.
func CreateDefaultDSCI(ctx context.Context, cli client.Client, _ cluster.Platform, appNamespace, monNamespace string) error {
log := logf.FromContext(ctx)
defaultDsciSpec := &dsciv1.DSCInitializationSpec{
ApplicationsNamespace: appNamespace,
Monitoring: dsciv1.Monitoring{
Expand Down Expand Up @@ -152,14 +153,14 @@ func CreateDefaultDSCI(ctx context.Context, cli client.Client, _ cluster.Platfor

switch {
case len(instances.Items) > 1:
ctrl.Log.Info("only one instance of DSCInitialization object is allowed. Please delete other instances.")
log.Info("only one instance of DSCInitialization object is allowed. Please delete other instances.")
return nil
case len(instances.Items) == 1:
// Do not patch/update if DSCI already exists.
ctrl.Log.Info("DSCInitialization resource already exists. It will not be updated with default DSCI.")
log.Info("DSCInitialization resource already exists. It will not be updated with default DSCI.")
return nil
case len(instances.Items) == 0:
ctrl.Log.Info("create default DSCI CR.")
log.Info("create default DSCI CR.")
err := cluster.CreateWithRetry(ctx, cli, defaultDsci, 1) // 1 min timeout
if err != nil {
return err
Expand Down Expand Up @@ -294,13 +295,14 @@ func deleteResources(ctx context.Context, c client.Client, resources *[]Resource
}

func deleteOneResource(ctx context.Context, c client.Client, res ResourceSpec) error {
log := logf.FromContext(ctx)
list := &unstructured.UnstructuredList{}
list.SetGroupVersionKind(res.Gvk)

err := c.List(ctx, list, client.InNamespace(res.Namespace))
if err != nil {
if errors.Is(err, &meta.NoKindMatchError{}) {
ctrl.Log.Info("CRD not found, will not delete " + res.Gvk.String())
log.Info("CRD not found, will not delete " + res.Gvk.String())
return nil
}
return fmt.Errorf("failed to list %s: %w", res.Gvk.Kind, err)
Expand All @@ -323,7 +325,7 @@ func deleteOneResource(ctx context.Context, c client.Client, res ResourceSpec) e
if err != nil {
return fmt.Errorf("failed to delete %s %s/%s: %w", res.Gvk.Kind, res.Namespace, item.GetName(), err)
}
ctrl.Log.Info("Deleted object " + item.GetName() + " " + res.Gvk.String() + "in namespace" + res.Namespace)
log.Info("Deleted object " + item.GetName() + " " + res.Gvk.String() + "in namespace" + res.Namespace)
}
}
}
Expand All @@ -332,6 +334,7 @@ func deleteOneResource(ctx context.Context, c client.Client, res ResourceSpec) e
}

func deleteDeprecatedResources(ctx context.Context, cli client.Client, namespace string, resourceList []string, resourceType client.ObjectList) error {
log := logf.FromContext(ctx)
var multiErr *multierror.Error
listOpts := &client.ListOptions{Namespace: namespace}
if err := cli.List(ctx, resourceType, listOpts); err != nil {
Expand All @@ -342,16 +345,16 @@ func deleteDeprecatedResources(ctx context.Context, cli client.Client, namespace
item := items.Index(i).Addr().Interface().(client.Object) //nolint:errcheck,forcetypeassert
for _, name := range resourceList {
if name == item.GetName() {
ctrl.Log.Info("Attempting to delete " + item.GetName() + " in namespace " + namespace)
log.Info("Attempting to delete " + item.GetName() + " in namespace " + namespace)
err := cli.Delete(ctx, item)
if err != nil {
if k8serr.IsNotFound(err) {
ctrl.Log.Info("Could not find " + item.GetName() + " in namespace " + namespace)
log.Info("Could not find " + item.GetName() + " in namespace " + namespace)
} else {
multiErr = multierror.Append(multiErr, err)
}
}
ctrl.Log.Info("Successfully deleted " + item.GetName())
log.Info("Successfully deleted " + item.GetName())
}
}
}
Expand All @@ -360,6 +363,7 @@ func deleteDeprecatedResources(ctx context.Context, cli client.Client, namespace

// Need to handle ServiceMonitor deletion separately as the generic function does not work for ServiceMonitors because of how the package is built.
func deleteDeprecatedServiceMonitors(ctx context.Context, cli client.Client, namespace string, resourceList []string) error {
log := logf.FromContext(ctx)
var multiErr *multierror.Error
listOpts := &client.ListOptions{Namespace: namespace}
servicemonitors := &monitoringv1.ServiceMonitorList{}
Expand All @@ -371,16 +375,16 @@ func deleteDeprecatedServiceMonitors(ctx context.Context, cli client.Client, nam
servicemonitor := servicemonitor
for _, name := range resourceList {
if name == servicemonitor.Name {
ctrl.Log.Info("Attempting to delete " + servicemonitor.Name + " in namespace " + namespace)
log.Info("Attempting to delete " + servicemonitor.Name + " in namespace " + namespace)
err := cli.Delete(ctx, servicemonitor)
if err != nil {
if k8serr.IsNotFound(err) {
ctrl.Log.Info("Could not find " + servicemonitor.Name + " in namespace " + namespace)
log.Info("Could not find " + servicemonitor.Name + " in namespace " + namespace)
} else {
multiErr = multierror.Append(multiErr, err)
}
}
ctrl.Log.Info("Successfully deleted " + servicemonitor.Name)
log.Info("Successfully deleted " + servicemonitor.Name)
}
}
}
Expand Down Expand Up @@ -451,33 +455,35 @@ func unsetOwnerReference(ctx context.Context, cli client.Client, instanceName st
}

func updateODCBiasMetrics(ctx context.Context, cli client.Client, instanceName string, oldRelease cluster.Release, odhObject *unstructured.Unstructured) error {
log := logf.FromContext(ctx)
// "from version" as oldRelease, if return "0.0.0" meaning running on 2.10- release/dummy CI build
// if oldRelease is lower than 2.14.0(e.g 2.13.x-a), flip disableBiasMetrics to false (even the field did not exist)
if oldRelease.Version.Minor < 14 {
ctrl.Log.Info("Upgrade force BiasMetrics to false in " + instanceName + " CR due to old release < 2.14.0")
log.Info("Upgrade force BiasMetrics to false in " + instanceName + " CR due to old release < 2.14.0")
// flip TrustyAI BiasMetrics to false (.spec.dashboardConfig.disableBiasMetrics)
disableBiasMetricsValue := []byte(`{"spec": {"dashboardConfig": {"disableBiasMetrics": false}}}`)
if err := cli.Patch(ctx, odhObject, client.RawPatch(types.MergePatchType, disableBiasMetricsValue)); err != nil {
return fmt.Errorf("error enable BiasMetrics in CR %s : %w", instanceName, err)
}
return nil
}
ctrl.Log.Info("Upgrade does not force BiasMetrics to false due to from release >= 2.14.0")
log.Info("Upgrade does not force BiasMetrics to false due to from release >= 2.14.0")
return nil
}

func updateODCModelRegistry(ctx context.Context, cli client.Client, instanceName string, oldRelease cluster.Release, odhObject *unstructured.Unstructured) error {
log := logf.FromContext(ctx)
// "from version" as oldRelease, if return "0.0.0" meaning running on 2.10- release/dummy CI build
// if oldRelease is lower than 2.14.0(e.g 2.13.x-a), flip disableModelRegistry to false (even the field did not exist)
if oldRelease.Version.Minor < 14 {
ctrl.Log.Info("Upgrade force ModelRegistry to false in " + instanceName + " CR due to old release < 2.14.0")
log.Info("Upgrade force ModelRegistry to false in " + instanceName + " CR due to old release < 2.14.0")
disableModelRegistryValue := []byte(`{"spec": {"dashboardConfig": {"disableModelRegistry": false}}}`)
if err := cli.Patch(ctx, odhObject, client.RawPatch(types.MergePatchType, disableModelRegistryValue)); err != nil {
return fmt.Errorf("error enable ModelRegistry in CR %s : %w", instanceName, err)
}
return nil
}
ctrl.Log.Info("Upgrade does not force ModelRegistry to false due to from release >= 2.14.0")
log.Info("Upgrade does not force ModelRegistry to false due to from release >= 2.14.0")
return nil
}

Expand All @@ -497,6 +503,7 @@ func RemoveLabel(ctx context.Context, cli client.Client, objectName string, labe
}

func deleteDeprecatedNamespace(ctx context.Context, cli client.Client, namespace string) error {
log := logf.FromContext(ctx)
foundNamespace := &corev1.Namespace{}
if err := cli.Get(ctx, client.ObjectKey{Name: namespace}, foundNamespace); err != nil {
if k8serr.IsNotFound(err) {
Expand Down Expand Up @@ -525,7 +532,7 @@ func deleteDeprecatedNamespace(ctx context.Context, cli client.Client, namespace
return fmt.Errorf("error getting pods from namespace %s: %w", namespace, err)
}
if len(podList.Items) != 0 {
ctrl.Log.Info("Skip deletion of namespace " + namespace + " due to running Pods in it")
log.Info("Skip deletion of namespace " + namespace + " due to running Pods in it")
return nil
}

Expand Down

0 comments on commit e2ed376

Please sign in to comment.