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

Add empty state when no serving platform is enabled #2109

Conversation

DaoDaoNoCode
Copy link
Member

@DaoDaoNoCode DaoDaoNoCode commented Nov 8, 2023

Closes #1994
Closes #1995

Description

There are several checks in this PR:

When the platforms are installed:
When we enter an unlabelled project and both disableKServe and disableModelMesh feature flags are set to true, you will get the empty state to tell you to ask for the administrator to enable platform selection.

Screenshot 2023-11-08 at 2 39 50 PM

When either platform is not installed:

On the cluster settings page:

  1. You will see the grey out checkbox which prevents you from enabling it if the platform is not installed on the cluster
  2. If you are the cluster admin, you will see the link to bring you to the DataScienceCluster resource page on the OpenShift console
Screenshot 2023-11-09 at 4 59 06 PM Screenshot 2023-11-09 at 4 59 55 PM
  1. If you don't have edit access to DataScienceCluster, the link button will be a plain text
Screenshot 2023-11-09 at 4 55 04 PM

On project details page:

If the project is labeled already but the serving platform in not installed at all, you will see a loading error

Screenshot 2023-11-09 at 5 03 50 PM

When both platforms are not installed:

You will see an error page on the global model serving page:

Screenshot 2023-11-09 at 5 05 27 PM

Of course, the cluster settings page grey both of them out because we only hide this section when you fully disable model serving.

Screenshot 2023-11-09 at 5 06 02 PM

How Has This Been Tested?

There are several parts of this:
First part:

  1. You need to make sure you get both model serving platforms installed on the cluster
  2. Go to the cluster settings or dashboard config, disable both of the available platforms
  3. Wait for 2 mins
  4. Create a new project and go to the model serving section, you will see the empty state as is shown above in the description
  5. The labeled projects should still be able to see the model/model server list

Second part:

  1. Now you can try to remove one of the serving platforms by changing the component from Managed to Removed, making sure the status changed to false, which means that platform is not available anymore
  2. Go to the cluster settings page, make sure the corresponding checkbox is greyed out
  3. Click on the help icon on the description, make sure you could see the text and access the link the the DataScienceCluster resource
  4. Try to impersonate a dashboard admin but not cluster admin user, you should only be able to see the plain text in the popover, but not the link
  5. Try to go to a project you created before, it's better to be a project with the serving platform labeled already and you removed the platform, you should see a loading error

Third part:

  1. Now you can removed the other serving platform, now none of the serving platform is available
  2. Go to the global model serving page, you should error an error state and cannot see any deployed model now

Test Impact

Update some feature flag related tests, and add an integration test to test the empty state.

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

@DaoDaoNoCode
Copy link
Member Author

@vconzola Could you check the screenshot in the description?

@DaoDaoNoCode DaoDaoNoCode linked an issue Nov 8, 2023 that may be closed by this pull request
@vconzola
Copy link

vconzola commented Nov 8, 2023

@DaoDaoNoCode LGTM.

@DaoDaoNoCode
Copy link
Member Author

@vconzola @kaedward I updated the descriptions and the screenshots based on the discussion this afternoon, could you check it again?

Copy link
Contributor

@lucferbux lucferbux left a comment

Choose a reason for hiding this comment

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

/lgtm

Tested an it's working, we can do a rundown next week with UX to spot mismatches but is high priority to integrate all the code today.

Copy link
Contributor

openshift-ci bot commented Nov 10, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lucferbux

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

@lucferbux lucferbux linked an issue Nov 10, 2023 that may be closed by this pull request
@openshift-merge-bot openshift-merge-bot bot merged commit 11e2189 into opendatahub-io:f/model-serving Nov 10, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Story]: Handle Cluster Component Installation [Story]: No serving platforms available view
3 participants