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

Fix retrieving OAuthClient Resource #165

Merged
merged 1 commit into from
Nov 18, 2022

Conversation

VaishnaviHire
Copy link

@VaishnaviHire VaishnaviHire commented Nov 16, 2022

This PR fixes the OAuthClient deletion issue. Note that this only happens in downstream Operator

Root Cause

Fix

  • This commit moves all Get calls for OAuthClient to List
  • Adds function to update OAuthClient resource incase there is a Stale object.

Testing Steps

This can be tested without Uninstallation as follows -

  • Install RHODS instance
     ./setup.sh quay.io/modh/rhods-operator-live-catalog:1.19.0-rhods-5836
    
  • Delete rhods-dashboard KfDef (wait for deletion to complete)
    oc delete kfdef rhods-dashboard -n redhat-ods-applications
    
  • Verify OAuthClient is deleted
    oc get oauthclient/dashboard-oauth-client 
    
  • To Verify OAuthClient update, create a dummy resource
    apiVersion: oauth.openshift.io/v1
    grantMethod: auto
    kind: OAuthClient
    metadata:
     name: dashboard-oauth-client
    secret: Zs6i7Q2AEjnHErMoxsjBSBmoyOD8Bwgd
    
  • Now re-create rhods-dashboard KfDef. The dashboard-oauth-client resource should be updated with latest secret.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • For commits that came from upstream, [UPSTREAM] has been prepended to the commit message.
  • JIRA link(s): https://issues.redhat.com/browse/RHODS-5836
  • The Jira story is acked.
  • Live build image:
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@VaishnaviHire
Copy link
Author

/retest

@openshift-ci
Copy link

openshift-ci bot commented Nov 16, 2022

@VaishnaviHire: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ci-index b639f46 link true /test ci-index
ci/prow/48-ci-index b639f46 link true /test 48-ci-index
ci/prow/modh-operator-e2e b639f46 link false /test modh-operator-e2e
ci/prow/images b639f46 link true /test images
ci/prow/48-modh-operator-e2e-48 b639f46 link false /test 48-modh-operator-e2e-48
ci/prow/48-images b639f46 link true /test 48-images

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@VaishnaviHire
Copy link
Author

VaishnaviHire commented Nov 16, 2022

/retest

@openshift-ci
Copy link

openshift-ci bot commented Nov 16, 2022

@VaishnaviHire: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test 48-ci-index
  • /test 48-images
  • /test 48-modh-unit-tests-48
  • /test ci-index
  • /test images
  • /test modh-unit-tests

The following commands are available to trigger optional jobs:

  • /test 48-modh-operator-e2e-48
  • /test modh-operator-e2e

Use /test all to run all jobs.

In response to this:

/retest aicoe-ci/build-check

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

@samuelvl samuelvl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

I confirm the oauthclient is removed after the dashboard KfDef is removed:

# OAuthClient exists
$ oc get oauthclient/dashboard-oauth-client 
NAME                     SECRET                             WWW-CHALLENGE   TOKEN-MAX-AGE   REDIRECT URIS
dashboard-oauth-client   es8XWimVObrTMwFJS6px0LOz8GNlHW32   false           default         https://rhods-dashboard-redhat-ods-applications.apps.samuvl.dev.datahub.redhat.com

# Delete KfDef
$ oc delete kfdef rhods-dashboard -n redhat-ods-applications
kfdef.kfdef.apps.kubeflow.org "rhods-dashboard" deleted

# OAuthClient is removed
$ oc get oauthclient/dashboard-oauth-client                 
Error from server (NotFound): oauthclients.oauth.openshift.io "dashboard-oauth-client" not found

@Jooho
Copy link

Jooho commented Nov 16, 2022

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Nov 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jooho, samuelvl, VaishnaviHire

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [VaishnaviHire,samuelvl]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@LaVLaS LaVLaS merged commit fa8987c into red-hat-data-services:master Nov 18, 2022
zdtsw added a commit to zdtsw/opendatahub-operator that referenced this pull request Feb 20, 2024
* sync: from main to rhoai-2.6 (red-hat-data-services#163)

* Add recording and alerting rules for TrustyAI

Update SOP url, RHODS to RHOAI

Fix trustyai-alerting.rules indentation

* fix(kserve): check on multiple depends operators if all pre-installed (opendatahub-io#744)

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* update: rewrite func to check operator (opendatahub-io#745)

creds to: @bartoszmajsak

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix(cleanup): cleans up dependant resources (opendatahub-io#748)

* feat: implements failing test for deletion using configmap

* fix(cleanup): cleans up dependant resources

The code responsible for cleaning up resources on cfg map presence was failing early due to operations on nil list instance of KfDef resources, leading to panic and restart of the pod making an impression that cleanup takes forever

* fix(reconcile): requeue only when actual error happens

Original code was always causing requeue as even if upgrade.OperatorUninstall(r.Client, r.RestConfig) resulted in nil error (success), it was wrapped in error with message error while operator uninstall: <nil>

* fix: reverts img placeholder in kustomize

* fix: removes commented out code

* fix(features): makes rest config loader more flexible (opendatahub-io#760)

* add table of contents to readme (opendatahub-io#769)

* Makefile: add clean target (opendatahub-io#733)

Add `make clean` which removes build artefacts. At the moment it's
./bin, ./odh-manifests/* and cover.out from test target.

Do not remove odh-manifests directory since it is commited to VCS.

In the recipe chown of $(LOCALBIN) since setup-envtest makes its dir
RO for some reason [1].

Related: opendatahub-io#696

[1] https://github.com/kubernetes-sigs/controller-runtime/blob/main/tools/setup-envtest/store/store.go#L191

Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>

* chore: cleanup owns on RS,Pod, daemonset, CRD (opendatahub-io#777)

* chore: cleanup owns on RS,Pod, daemonset, CRD
* cleanup: remove developmentconfig not in use
* chore: move rbac into one file and remove duplicated one

---------
Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* update(kserve): add monitoring logic (opendatahub-io#782)

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* chore(mesh): Use third party service account tokens (opendatahub-io#767)

This should allow Service Mesh to run on any OpenShift flavor.

Fixes opendatahub-io/kserve#138

Signed-off-by: Edgar Hernández <23639005+israel-hdez@users.noreply.github.com>

* chore: change log info for monitoring patch namespace (opendatahub-io#787)

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix devflags empty object (opendatahub-io#659)

- change DefFlags to pointer type , see reason from example: https://www.sohamkamani.com/golang/omitempty/
- change both in DSC and DSCI

* feature: add support for unmanaged and remove for servicemesh and serverless (opendatahub-io#781)

* feature: add support for unmanaged and remove for servicemesh and
serverless

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* Update components/kserve/kserve.go

Co-authored-by: Bartosz Majsak <bartosz.majsak@gmail.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Co-authored-by: Bartosz Majsak <bartosz.majsak@gmail.com>

* fix(nilpointer): when more than one DSCI CR in cluster (opendatahub-io#756)

* fix(nilpointer): when more than one DSCI CR in cluster

- use the one with ealiest timestamp as the default one
- suggest user to delete other CR but only use default one
- only set the extra/wrong DSCI in Error status

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Co-authored-by: Bartosz Majsak <bartosz.majsak@gmail.com>

* chore: change label name (opendatahub-io#790)

- use a more generaic label than bind to namespace name

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* Remove lavlas from OWNERS (opendatahub-io#791)

* fix(builder): initializes rest config before invoking any partial builder (opendatahub-io#792)

This way we can obtain interact with the cluster from partial builders instead of defering it to Apply phase.

* chore: shifts FeatureTracker creation to Feature's Apply phase (opendatahub-io#795)

* fix(mesh): disables default network policy management (opendatahub-io#798)

* fix: disables default network policy management

* fix: returns correct error

Co-authored-by: Wen Zhou <wenzhou@redhat.com>

---------

Co-authored-by: Wen Zhou <wenzhou@redhat.com>

* feat(linters): Enable all linters by default. (opendatahub-io#692)

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
Signed-off-by: Edgar Hernández <23639005+israel-hdez@users.noreply.github.com>
Co-authored-by: Rui Vieira <ruidevieira@googlemail.com>
Co-authored-by: Bartosz Majsak <bartosz.majsak@gmail.com>
Co-authored-by: Ajay Jaganathan <36824134+AjayJagan@users.noreply.github.com>
Co-authored-by: Yauheni Kaliuta <ykaliuta@redhat.com>
Co-authored-by: Edgar Hernández <ehernand@redhat.com>
Co-authored-by: Landon LaSmith <2432396+LaVLaS@users.noreply.github.com>

* update: rebranding for rhoai from rhods (red-hat-data-services#160)

* update: rebranding for rhoai from rhods

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* Update: new icon data and link to dashboard icon

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix: couple of changes (red-hat-data-services#164)

* fix: couple of changes

- change from 2.5 missing on 2.6
- version/branch from components
- generated bundle with rbac
- linter

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* [cherry-pick]: odh opendatahub-io#809

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* update(trustyai): set component by default as Managed as GA in 2.6

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
Signed-off-by: Edgar Hernández <23639005+israel-hdez@users.noreply.github.com>
Co-authored-by: Rui Vieira <ruidevieira@googlemail.com>
Co-authored-by: Bartosz Majsak <bartosz.majsak@gmail.com>
Co-authored-by: Ajay Jaganathan <36824134+AjayJagan@users.noreply.github.com>
Co-authored-by: Yauheni Kaliuta <ykaliuta@redhat.com>
Co-authored-by: Edgar Hernández <ehernand@redhat.com>
Co-authored-by: Landon LaSmith <2432396+LaVLaS@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants