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

Handle Removed but not Disabled DS Pipelines #2018

Merged

Conversation

andrewballantyne
Copy link
Member

@andrewballantyne andrewballantyne commented Oct 26, 2023

Closes: #1870

Description

When DSP is disabled in v2 Operator, the context fails and blocks the rendering of DS Projects details view.

Screenshots / Flow

Two users:

  1. Cluster Admin - The cluster admin can read 404 normally when an API is not available (404 error)
  2. Regular user (eg. admin with no special permissions) - The regular user cannot read API from one that does not exist (403 error)
    • This will be done through impersonate, same difference on cluster (the screenshot has a white button in the header)

Note: There are a lot of screenshots to show each state, so I added collapsible sections. Expand each one to see the screenshots. Two sets will be provided in each section. One for the cluster-admin and one for the "regular user".

DS Projects - No Operator installed & No DS Pipelines Component

Screenshots

Screenshot 2023-10-26 at 10 56 50 AM
Screenshot 2023-10-26 at 10 57 15 AM


DS Projects - Operator installed & No DS Pipelines Component

Screenshots

Screenshot 2023-10-26 at 12 39 56 PM
Screenshot 2023-10-26 at 12 40 34 PM

Global Screens (Pipeline & Run) - Operator installed & No DS Pipelines Component

Screenshots

Screenshot 2023-10-26 at 11 01 22 AM
Screenshot 2023-10-26 at 11 01 11 AM

Screenshot 2023-10-26 at 11 00 29 AM
Screenshot 2023-10-26 at 11 00 42 AM


DS Projects - Everything installed

Screenshots

Screenshot 2023-10-26 at 12 56 28 PM
Screenshot 2023-10-26 at 12 57 38 PM

Global Screens (Pipeline & Run) - Everything installed

Screenshots

Screenshot 2023-10-26 at 12 56 48 PM
Screenshot 2023-10-26 at 12 57 04 PM
Screenshot 2023-10-26 at 12 57 49 PM
Screenshot 2023-10-26 at 12 57 58 PM

How Has This Been Tested?

Three layers of testing here

  • First layer is nothing installed, no operator & no component << Was the bug, installing the operator got a different error
  • Second layer is just operator, no component << Once the first layer is removed, the error changes
  • Third layer is both operator & component << Once the second layer is removed, we successfully render

Normal flows continue after this.

Test Impact

Deferring tests to #2010. This is a temporary fix to still show errors in the UI -- but nicer. To properly support components against our feature flags we need 2010, where tests should be included.

Request review criteria:

Self checklist (all need to be checked):

  • The developer has manually tested the changes and verified that the changes work
  • Commits have been squashed into descriptive, self-contained units of work (e.g. 'WIP' and 'Implements feedback' style messages have been removed)
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has added tests or explained why testing cannot be added (unit tests & storybook for related changes)

If you have UI changes:

  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change (find relevant UX in the SMEs section).

After the PR is posted & before it merges:

  • The developer has tested their solution on a cluster by using the image produced by the PR to main

@christianvogt
Copy link
Contributor

scenarios work as described
/lgtm
/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt

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:

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

@christianvogt christianvogt added the pr/no-tests-allowed Added by an official approver - this PR is allowed no tests. Omitted, a test must accompany the PR label Oct 26, 2023
@openshift-ci openshift-ci bot merged commit 265b0b2 into opendatahub-io:main Oct 26, 2023
6 checks passed
@manosnoam
Copy link
Contributor

Verified on RHOAI 2.5

@andrewballantyne andrewballantyne deleted the dsp-removed-not-disabled branch June 26, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm pr/no-tests-allowed Added by an official approver - this PR is allowed no tests. Omitted, a test must accompany the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Operator V2] Access to DS Projects fails if DS Pipelines are disabled from DSC
3 participants