Check serving runtime kind at creation time

[DaoDaoNoCode]

Closes #1353

Description

1. Check the format of the custom serving runtime at the creation/edit time, and prompt an alert if the requirement is not met

2. If the template is accidentally changed to an invalid format (not through the dashboard UI because the UI will prevent that from happening), filter out the template selection at the serving runtime creation modal, because it makes no sense if you select a serving runtime template which is invalid

How Has This Been Tested?

1. Go to the custom serving runtimes page
2. Copy an OOTB serving runtime
3. Change the kind to ServingRuntim or any value that's not ServingRuntime
4. Click submit, you will see the error message
5. Set the valid kind but remove the container or supportedModelFormats field in the spec
6. Click submit again, the same error message will show
7. Reset everything back to the valid value and duplicate the serving runtime
8. Edit the YAML of the new duplicated template in the OpenShift console (You can find it by searching the resource Template in the dashboard namespace), change the kind in the objects from ServingRuntime to an invalid value in that template resource
9. Go to the project details page, click Add model server, and make sure you cannot see the template in the template selection dropdown menu

Test Impact

Add an invalid mock data for the template, and make sure it won't show in the template selection dropdown when creating a model server

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]

@kaedward @vconzola Hi, could you check the wording in the screenshot? We want to tell the user when they create/edit the custom serving runtimes, the kind needs to be ServingRuntime, and the spec.container and spec.supportedModelFormats are required fields.

[vconzola]

@DaoDaoNoCode I would separate the errors into two messages, and only display the appropriate message(s) when the condition is true. But @kaedward should chime in on the that. I have a question about the interaction. When is the error displayed - as soon as the user edits the field(s) or when they click Update?

[DaoDaoNoCode]

@vconzola The error happens only when users click the update button. We don't validate anything when they type.

[vconzola]

@DaoDaoNoCode Given that the error isn't recognized until the user selects Update, can we break the error into three distinct messages depending on what's wrong? For example,

• Title = "Invalid parameter", Text = "The kind: must be ServingRuntime." or
• Title = "Missing parameter", Text = "The container: field is required." or
• Title = "Missing parameter", Text = "The supportedModelFormats: field is required."

@kaedward What do you think?

[kaedward]

@vconzola I like that approach, it sets a standard for these single errors so that we don't have to write new unique messages for each combination of them. I made a few edits, since I don't necessarily think these kinds of errors need to be written in full sentences. I think we could also remove the titles since the error messages are so concise... would make it easier to scan if the user happens to have a lot of errors.

Title = "Invalid parameter", Text = "kind: must be ServingRuntime."
Title = "Missing parameter", Text = "container: is required."
Title = "Missing parameter", Text = "supportedModelFormats: is required."

If we can't do that... Is the "Error" alert header necessary? I would prefer not to have a header here if we can help it, since it's basically helper/validation text. Here's a suggestion for the body:

Enter ServingRuntime in the Kind field, and complete the required fields containers and supportedModelFormats.

[DaoDaoNoCode]

@vconzola @kaedward Thanks for the feedback, the title is a required field for the alert, I can set message as the title, and don't set any message in the body, and it looks all bold font, is that ok?

[vconzola]

@DaoDaoNoCode I think the idea is that we should have separate messages for each error, using the text @kaedward proposed.

• Title = "Invalid parameter", Text = "kind: must be ServingRuntime."
• Title = "Missing parameter", Text = "container: is required."
• Title = "Missing parameter", Text = "supportedModelFormats: is required."

Katie - Correct me if I misunderstood.

[DaoDaoNoCode]

@vconzola It's a little bit hard to customize the title actually, do you have any suggestions on the general title we should use? The body message can be fully customized of course.

[kaedward]

@vconzola It's a little bit hard to customize the title actually, do you have any suggestions on the general title we should use? The body message can be fully customized of course.

I'm not sure I understand what you mean by general title - is it not possible to have 3 separate error messages like @vconzola suggested above?

[DaoDaoNoCode]

@kaedward It's definitely doable, let me try to hack this tomorrow. Thanks for the feedback.

[DaoDaoNoCode]

@vconzola @kaedward Updated, please check.

[vconzola]

@DaoDaoNoCode Perfect!

[lucferbux]

/lgtm

Ok, validation works great, it seems to be checking the main 4 issues and the code looks great

[openshift-ci]

[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:

• OWNERS [lucferbux]

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