From 22e94b9b4be9758f332d729bf4879456b2c2edc2 Mon Sep 17 00:00:00 2001 From: VaishnaviHire Date: Wed, 16 Oct 2024 07:49:56 -0400 Subject: [PATCH] Fix linters --- components/dashboard/dashboard.go | 22 +- .../components/codeflare_controller.go | 2 +- .../components/dashboard_controller.go | 14 +- .../datasciencepipelines_controller.go | 2 +- controllers/components/kserve_controller.go | 2 +- controllers/components/kueue_controller.go | 2 +- .../components/modelmeshserving_controller.go | 2 +- .../components/modelregistry_controller.go | 2 +- controllers/components/ray_controller.go | 2 +- controllers/components/suite_test.go | 3 +- .../components/trainingoperator_controller.go | 2 +- controllers/components/trustyai_controller.go | 2 +- .../components/workbenches_controller.go | 2 +- .../datasciencecluster_controller.go | 2 +- main.go | 18 -- pkg/controller/actions/action_test.go | 13 +- .../actions/action_update_status_test.go | 208 +++++++----------- 17 files changed, 121 insertions(+), 179 deletions(-) diff --git a/components/dashboard/dashboard.go b/components/dashboard/dashboard.go index 9953047a3c6..c0bb1ed37c5 100644 --- a/components/dashboard/dashboard.go +++ b/components/dashboard/dashboard.go @@ -4,7 +4,7 @@ package dashboard // -//import ( +// import ( // "context" // "errors" // "fmt" @@ -24,7 +24,7 @@ package dashboard // "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" //) // -//var ( +// var ( // ComponentNameUpstream = "dashboard" // PathUpstream = deploy.DefaultManifestPath + "/" + ComponentNameUpstream + "/odh" // @@ -37,15 +37,15 @@ package dashboard //) // //// Verifies that Dashboard implements ComponentInterface. -//var _ components.ComponentInterface = (*Dashboard)(nil) +// var _ components.ComponentInterface = (*Dashboard)(nil) // //// Dashboard struct holds the configuration for the Dashboard component. //// +kubebuilder:object:generate=true -//type Dashboard struct { +// type Dashboard struct { // components.Component `json:""` //} // -//func (d *Dashboard) Init(ctx context.Context, platform cluster.Platform) error { +// func (d *Dashboard) Init(ctx context.Context, platform cluster.Platform) error { // log := logf.FromContext(ctx).WithName(ComponentNameUpstream) // // imageParamMap := map[string]string{ @@ -65,7 +65,7 @@ package dashboard // return nil //} // -//func (d *Dashboard) OverrideManifests(ctx context.Context, platform cluster.Platform) error { +// func (d *Dashboard) OverrideManifests(ctx context.Context, platform cluster.Platform) error { // // If devflags are set, update default manifests path // if len(d.DevFlags.Manifests) != 0 { // manifestConfig := d.DevFlags.Manifests[0] @@ -79,18 +79,18 @@ package dashboard // return nil //} // -//func (d *Dashboard) GetComponentName() string { +// func (d *Dashboard) GetComponentName() string { // return ComponentNameUpstream //} // -//func (d *Dashboard) ReconcileComponent(ctx context.Context, +// func (d *Dashboard) ReconcileComponent(ctx context.Context, // cli client.Client, // l logr.Logger, // owner metav1.Object, // dscispec *dsciv1.DSCInitializationSpec, // platform cluster.Platform, // currentComponentExist bool, -//) error { +// ) error { // entryPath := DefaultPath // enabled := d.GetManagementState() == operatorv1.Managed // monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed @@ -184,7 +184,7 @@ package dashboard // } //} // -//func updateKustomizeVariable(ctx context.Context, cli client.Client, platform cluster.Platform, dscispec *dsciv1.DSCInitializationSpec) (map[string]string, error) { +// func updateKustomizeVariable(ctx context.Context, cli client.Client, platform cluster.Platform, dscispec *dsciv1.DSCInitializationSpec) (map[string]string, error) { // adminGroups := map[cluster.Platform]string{ // cluster.SelfManagedRhods: "rhods-admins", // cluster.ManagedRhods: "dedicated-admins", @@ -217,7 +217,7 @@ package dashboard // }, nil //} // -//func (d *Dashboard) cleanOauthClient(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec, currentComponentExist bool, l logr.Logger) error { +// func (d *Dashboard) cleanOauthClient(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec, currentComponentExist bool, l logr.Logger) error { // // Remove previous oauth-client secrets // // Check if component is going from state of `Not Installed --> Installed` // // Assumption: Component is currently set to enabled diff --git a/controllers/components/codeflare_controller.go b/controllers/components/codeflare_controller.go index c59aebc5057..2fbcbd0c35d 100644 --- a/controllers/components/codeflare_controller.go +++ b/controllers/components/codeflare_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// CodeFlareReconciler reconciles a CodeFlare object +// CodeFlareReconciler reconciles a CodeFlare object. type CodeFlareReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/components/dashboard_controller.go b/controllers/components/dashboard_controller.go index cba88746ec6..e8084df11d6 100644 --- a/controllers/components/dashboard_controller.go +++ b/controllers/components/dashboard_controller.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "k8s.io/utils/pointer" "path/filepath" appsv1 "k8s.io/api/apps/v1" @@ -177,11 +176,7 @@ func watchDashboardResources(_ context.Context, a client.Object) []reconcile.Req var dashboardPredicates = predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { - if e.Object.GetObjectKind().GroupVersionKind().Kind == gvk.Dashboard.Kind { - return true - } - // Reconcile not needed during creation - return false + return e.Object.GetObjectKind().GroupVersionKind().Kind == gvk.Dashboard.Kind }, DeleteFunc: func(e event.DeleteEvent) bool { if e.Object.GetObjectKind().GroupVersionKind().Kind == gvk.Dashboard.Kind { @@ -368,6 +363,8 @@ func (a *DeployComponentAction) Execute(ctx context.Context, rr *odhtypes.Reconc switch rr.Platform { case cluster.SelfManagedRhods, cluster.ManagedRhods: // anaconda + blckownerDel := true + ctrlr := true err := cluster.CreateSecret( ctx, rr.Client, @@ -379,8 +376,8 @@ func (a *DeployComponentAction) Execute(ctx context.Context, rr *odhtypes.Reconc Kind: rr.Instance.GetObjectKind().GroupVersionKind().Kind, Name: rr.Instance.GetName(), UID: rr.Instance.GetUID(), - Controller: pointer.Bool(true), - BlockOwnerDeletion: pointer.Bool(true), + Controller: &ctrlr, + BlockOwnerDeletion: &blckownerDel, }), cluster.WithLabels( labels.ComponentName, ComponentName, @@ -394,6 +391,7 @@ func (a *DeployComponentAction) Execute(ctx context.Context, rr *odhtypes.Reconc } name = ComponentNameDownstream + default: } err = deploy.DeployManifestsFromPathWithLabels(ctx, rr.Client, rr.Instance, path, rr.DSCI.Spec.ApplicationsNamespace, name, true, map[string]string{ diff --git a/controllers/components/datasciencepipelines_controller.go b/controllers/components/datasciencepipelines_controller.go index 4ec5f73c60a..e59557dfae1 100644 --- a/controllers/components/datasciencepipelines_controller.go +++ b/controllers/components/datasciencepipelines_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// DataSciencePipelinesReconciler reconciles a DataSciencePipelines object +// DataSciencePipelinesReconciler reconciles a DataSciencePipelines object. type DataSciencePipelinesReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/components/kserve_controller.go b/controllers/components/kserve_controller.go index 03dd01fc5ac..5379bca4d46 100644 --- a/controllers/components/kserve_controller.go +++ b/controllers/components/kserve_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// KserveReconciler reconciles a Kserve object +// KserveReconciler reconciles a Kserve object. type KserveReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/components/kueue_controller.go b/controllers/components/kueue_controller.go index 0743383ad34..499a882a99a 100644 --- a/controllers/components/kueue_controller.go +++ b/controllers/components/kueue_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// KueueReconciler reconciles a Kueue object +// KueueReconciler reconciles a Kueue object. type KueueReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/components/modelmeshserving_controller.go b/controllers/components/modelmeshserving_controller.go index 9681bb8d451..e7a29bc8131 100644 --- a/controllers/components/modelmeshserving_controller.go +++ b/controllers/components/modelmeshserving_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// ModelMeshServingReconciler reconciles a ModelMeshServing object +// ModelMeshServingReconciler reconciles a ModelMeshServing object. type ModelMeshServingReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/components/modelregistry_controller.go b/controllers/components/modelregistry_controller.go index 475ddbf29c7..fa97bc6177d 100644 --- a/controllers/components/modelregistry_controller.go +++ b/controllers/components/modelregistry_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// ModelRegistryReconciler reconciles a ModelRegistry object +// ModelRegistryReconciler reconciles a ModelRegistry object. type ModelRegistryReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/components/ray_controller.go b/controllers/components/ray_controller.go index 6b13b2a5f01..e1ea8524bf3 100644 --- a/controllers/components/ray_controller.go +++ b/controllers/components/ray_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// RayReconciler reconciles a Ray object +// RayReconciler reconciles a Ray object. type RayReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/components/suite_test.go b/controllers/components/suite_test.go index f27bb88f16b..29231eb7dbe 100644 --- a/controllers/components/suite_test.go +++ b/controllers/components/suite_test.go @@ -14,8 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -package components +package components_test +//revive:disable:dot-imports import ( "path/filepath" "testing" diff --git a/controllers/components/trainingoperator_controller.go b/controllers/components/trainingoperator_controller.go index 9e8d7b790d6..e6b53b69925 100644 --- a/controllers/components/trainingoperator_controller.go +++ b/controllers/components/trainingoperator_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// TrainingOperatorReconciler reconciles a TrainingOperator object +// TrainingOperatorReconciler reconciles a TrainingOperator object. type TrainingOperatorReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/components/trustyai_controller.go b/controllers/components/trustyai_controller.go index 579175d979e..0e3f334c6ae 100644 --- a/controllers/components/trustyai_controller.go +++ b/controllers/components/trustyai_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// TrustyAIReconciler reconciles a TrustyAI object +// TrustyAIReconciler reconciles a TrustyAI object. type TrustyAIReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/components/workbenches_controller.go b/controllers/components/workbenches_controller.go index dc200a112f2..bcb4096558d 100644 --- a/controllers/components/workbenches_controller.go +++ b/controllers/components/workbenches_controller.go @@ -27,7 +27,7 @@ import ( componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" ) -// WorkbenchesReconciler reconciles a Workbenches object +// WorkbenchesReconciler reconciles a Workbenches object. type WorkbenchesReconciler struct { client.Client Scheme *runtime.Scheme diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 4f5f356e4d3..c20ab492a5b 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -83,7 +83,7 @@ const ( // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. -func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:maintidx,gocyclo +func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:maintidx log := r.Log log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name) diff --git a/main.go b/main.go index 8ad711c6543..ce7ba4bdeea 100644 --- a/main.go +++ b/main.go @@ -21,7 +21,6 @@ import ( "flag" "os" - "github.com/hashicorp/go-multierror" addonv1alpha1 "github.com/openshift/addon-operator/apis/addons/v1alpha1" ocappsv1 "github.com/openshift/api/apps/v1" //nolint:importas //reason: conflicts with appsv1 "k8s.io/api/apps/v1" buildv1 "github.com/openshift/api/build/v1" @@ -81,7 +80,6 @@ var ( ) func init() { //nolint:gochecknoinits - utilruntime.Must(componentsv1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme utilruntime.Must(clientgoscheme.AddToScheme(scheme)) @@ -108,22 +106,6 @@ func init() { //nolint:gochecknoinits utilruntime.Must(operatorv1.Install(scheme)) } -func initComponents(ctx context.Context, p cluster.Platform) error { - var errs *multierror.Error - var dummyDSC = &dscv1.DataScienceCluster{} - - components, err := dummyDSC.GetComponents() - if err != nil { - return err - } - - for _, c := range components { - errs = multierror.Append(errs, c.Init(ctx, p)) - } - - return errs.ErrorOrNil() -} - func main() { //nolint:funlen,maintidx var metricsAddr string var enableLeaderElection bool diff --git a/pkg/controller/actions/action_test.go b/pkg/controller/actions/action_test.go index 75dd098fde9..b5ec76056c3 100644 --- a/pkg/controller/actions/action_test.go +++ b/pkg/controller/actions/action_test.go @@ -13,18 +13,19 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" ) -func NewFakeClient(objs ...ctrlClient.Object) ctrlClient.WithWatch { +func NewFakeClient(objs ...ctrlClient.Object) ctrlClient.WithWatch { //nolint:ireturn scheme := runtime.NewScheme() utilruntime.Must(corev1.AddToScheme(scheme)) utilruntime.Must(appsv1.AddToScheme(scheme)) fakeMapper := meta.NewDefaultRESTMapper(scheme.PreferredVersionAllGroups()) for gvk := range scheme.AllKnownTypes() { - switch { - // TODO: add cases for cluster scoped - default: - fakeMapper.Add(gvk, meta.RESTScopeNamespace) - } + fakeMapper.Add(gvk, meta.RESTScopeNamespace) + // switch { + //// TODO: add cases for cluster scoped + //default: + // fakeMapper.Add(gvk, meta.RESTScopeNamespace) + //} } return fake.NewClientBuilder(). diff --git a/pkg/controller/actions/action_update_status_test.go b/pkg/controller/actions/action_update_status_test.go index 2e3a38f639d..2d3fe8a07cb 100644 --- a/pkg/controller/actions/action_update_status_test.go +++ b/pkg/controller/actions/action_update_status_test.go @@ -8,6 +8,7 @@ import ( "github.com/rs/xid" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" @@ -22,142 +23,101 @@ import ( . "github.com/onsi/gomega" ) -func TestUpdateStatusActionNotReady(t *testing.T) { - g := NewWithT(t) - - ctx := context.Background() - ns := xid.New().String() - - client := NewFakeClient( - &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gvk.Deployment.GroupVersion().String(), - Kind: gvk.Deployment.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "my-deployment", - Namespace: ns, - Labels: map[string]string{ - labels.K8SCommon.PartOf: "foo", - }, - }, - Status: appsv1.DeploymentStatus{ - Replicas: 1, - ReadyReplicas: 0, +func TestUpdateStatusAction(t *testing.T) { + _ = NewWithT(t) + + testCases := []struct { + name string + deployments []appsv1.Deployment + expectedStatus metav1.ConditionStatus + expectedReason string + additionalAssertions func(*testing.T, *types.ReconciliationRequest) + }{ + { + name: "Not Ready - One deployment not ready", + deployments: []appsv1.Deployment{ + createDeployment("my-deployment", 1, 0), + createDeployment("my-deployment-2", 1, 1), }, + expectedStatus: metav1.ConditionFalse, + expectedReason: actions.DeploymentsNotReadyReason, }, - &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gvk.Deployment.GroupVersion().String(), - Kind: gvk.Deployment.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "my-deployment-2", - Namespace: ns, - Labels: map[string]string{ - labels.K8SCommon.PartOf: "foo", - }, - }, - Status: appsv1.DeploymentStatus{ - Replicas: 1, - ReadyReplicas: 1, + { + name: "Ready - All deployments ready", + deployments: []appsv1.Deployment{ + createDeployment("my-deployment", 1, 1), + createDeployment("my-deployment-2", 2, 2), }, + expectedStatus: metav1.ConditionTrue, + expectedReason: actions.ReadyReason, }, - ) - - action := actions.NewUpdateStatusAction( - ctx, - actions.WithUpdateStatusLabel(labels.K8SCommon.PartOf, "foo")) - - rr := types.ReconciliationRequest{ - Client: client, - Instance: &componentsv1.Dashboard{}, - DSCI: &dsciv1.DSCInitialization{Spec: dsciv1.DSCInitializationSpec{ApplicationsNamespace: ns}}, - DSC: &dscv1.DataScienceCluster{}, - Platform: cluster.OpenDataHub, } - err := action.Execute(ctx, &rr) - g.Expect(err).ShouldNot(HaveOccurred()) - - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(rr.Instance).Should( - WithTransform( - ExtractStatusCondition(status.ConditionTypeReady), - gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(actions.DeploymentsNotReadyReason), - }), - ), - ) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + ns := xid.New().String() + + cli := NewFakeClient(createObjectList(tc.deployments)...) + + action := actions.NewUpdateStatusAction( + ctx, + actions.WithUpdateStatusLabel(labels.K8SCommon.PartOf, "foo")) + + rr := types.ReconciliationRequest{ + Client: cli, + Instance: &componentsv1.Dashboard{}, + DSCI: &dsciv1.DSCInitialization{Spec: dsciv1.DSCInitializationSpec{ApplicationsNamespace: ns}}, + DSC: &dscv1.DataScienceCluster{}, + Platform: cluster.OpenDataHub, + } + + err := action.Execute(ctx, &rr) + g.Expect(err).ShouldNot(HaveOccurred()) + + g.Expect(rr.Instance).Should( + WithTransform( + ExtractStatusCondition(status.ConditionTypeReady), + gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ + "Status": Equal(tc.expectedStatus), + "Reason": Equal(tc.expectedReason), + }), + ), + ) + + if tc.additionalAssertions != nil { + tc.additionalAssertions(t, &rr) + } + }) + } } -func TestUpdateStatusActionReady(t *testing.T) { - g := NewWithT(t) - - ctx := context.Background() - ns := xid.New().String() +// Helper functions - client := NewFakeClient( - &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gvk.Deployment.GroupVersion().String(), - Kind: gvk.Deployment.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "my-deployment", - Namespace: ns, - Labels: map[string]string{ - labels.K8SCommon.PartOf: "foo", - }, - }, - Status: appsv1.DeploymentStatus{ - Replicas: 1, - ReadyReplicas: 1, - }, +func createDeployment(name string, replicas, readyReplicas int32) appsv1.Deployment { + return appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + APIVersion: gvk.Deployment.GroupVersion().String(), + Kind: gvk.Deployment.Kind, }, - &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gvk.Deployment.GroupVersion().String(), - Kind: gvk.Deployment.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "my-deployment-2", - Namespace: ns, - Labels: map[string]string{ - labels.K8SCommon.PartOf: "foo", - }, - }, - Status: appsv1.DeploymentStatus{ - Replicas: 1, - ReadyReplicas: 1, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + labels.K8SCommon.PartOf: "foo", }, }, - ) - - action := actions.NewUpdateStatusAction( - ctx, - actions.WithUpdateStatusLabel(labels.K8SCommon.PartOf, "foo")) - - rr := types.ReconciliationRequest{ - Client: client, - Instance: &componentsv1.Dashboard{}, - DSCI: &dsciv1.DSCInitialization{Spec: dsciv1.DSCInitializationSpec{ApplicationsNamespace: ns}}, - DSC: &dscv1.DataScienceCluster{}, - Platform: cluster.OpenDataHub, + Status: appsv1.DeploymentStatus{ + Replicas: replicas, + ReadyReplicas: readyReplicas, + }, } +} - err := action.Execute(ctx, &rr) - g.Expect(err).ShouldNot(HaveOccurred()) - - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(rr.Instance).Should( - WithTransform( - ExtractStatusCondition(status.ConditionTypeReady), - gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ - "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(actions.ReadyReason), - }), - ), - ) +func createObjectList(deployments []appsv1.Deployment) []client.Object { + objects := make([]client.Object, len(deployments)) + for i := range deployments { + objects[i] = &deployments[i] + } + return objects }