Follow-up fixes for model registry deployment modal

frontend/src/__tests__/cypress/cypress/pages/modelServing.ts

@@ -121,6 +121,12 @@ class InferenceServiceModal extends Modal {
       .findByRole('button', { name: 'Options menu' });
   }
 
+  selectExistingConnectionSelectOptionByResourceName(name: string) {
+    this.findExistingConnectionSelect()
+      .findSelectOptionByTestId(`inference-service-data-connection ${name}`)
+      .click();
+  }
+
   findLocationNameInput() {
     return this.find().findByTestId('field Name');
   }

frontend/src/__tests__/cypress/cypress/support/commands/application.ts

@@ -53,6 +53,12 @@ declare global {
        * @param name the name of the option
        */
       findSelectOption: (name: string | RegExp) => Cypress.Chainable<JQuery>;
+      /**
+       * Finds a patternfly select option by first opening the select menu if not already opened.
+       *
+       * @param testId the name of the option
+       */
+      findSelectOptionByTestId: (testId: string) => Cypress.Chainable<JQuery>;
 
       /**
        * Shortcut to first clear the previous value and then type text into DOM element.
@@ -199,6 +205,16 @@ Cypress.Commands.add('findSelectOption', { prevSubject: 'element' }, (subject, n
   });
 });
 
+Cypress.Commands.add('findSelectOptionByTestId', { prevSubject: 'element' }, (subject, testId) => {
+  Cypress.log({ displayName: 'findSelectOptionByTestId', message: testId });
+  return cy.wrap(subject).then(($el) => {
+    if ($el.find('[aria-expanded=false]').addBack().length) {
+      cy.wrap($el).click();
+    }
+    return cy.wrap($el).parent().findByTestId(testId);
+  });
+});
+
 Cypress.Commands.add('fill', { prevSubject: 'optional' }, (subject, text, options) => {
   cy.wrap(subject).clear();
   return cy.wrap(subject).type(text, options);

frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDeploy.cy.ts

@@ -287,7 +287,7 @@ describe('Deploy model version', () => {
 
     // Validate data connection section
     kserveModal.findExistingDataConnectionOption().should('be.checked');
-    kserveModal.findExistingConnectionSelect().should('contain.text', 'Test Secret');
+    kserveModal.findExistingConnectionSelect().should('contain.text', 'Test SecretRecommended');
     kserveModal.findLocationPathInput().should('have.value', 'demo-models/test-path');
   });
 
@@ -310,6 +310,14 @@ describe('Deploy model version', () => {
           endPoint: 'dGVzdC1lbmRwb2ludA==',
           region: 'dGVzdC1yZWdpb24=',
         }),
+        mockSecretK8sResource({
+          name: 'test-secret-not-match',
+          displayName: 'Test Secret Not Match',
+          namespace: 'kserve-project',
+          s3Bucket: 'dGVzdC1idWNrZXQ=',
+          endPoint: 'dGVzdC1lbmRwb2ludC1ub3QtbWF0Y2g=', // endpoint not match
+          region: 'dGVzdC1yZWdpb24=',
+        }),
       ]),
     );
 
@@ -322,5 +330,15 @@ describe('Deploy model version', () => {
     kserveModal.findExistingDataConnectionOption().should('be.checked');
     kserveModal.findExistingConnectionSelect().should('contain.text', 'Select...');
     kserveModal.findLocationPathInput().should('have.value', 'demo-models/test-path');
+
+    // Make sure recommended label is there
+    kserveModal.selectExistingConnectionSelectOptionByResourceName('test-secret');
+    kserveModal.findExistingConnectionSelect().should('contain.text', 'Recommended');
+
+    kserveModal.selectExistingConnectionSelectOptionByResourceName('test-secret-2');
+    kserveModal.findExistingConnectionSelect().should('contain.text', 'Recommended');
+
+    kserveModal.selectExistingConnectionSelectOptionByResourceName('test-secret-not-match');
+    kserveModal.findExistingConnectionSelect().should('not.contain.text', 'Recommended');
   });
 });

frontend/src/pages/modelRegistry/screens/RegisteredModels/useProjectErrorForRegisteredModel.ts

@@ -4,31 +4,37 @@ import { ServingRuntimePlatform } from '~/types';
 const useProjectErrorForRegisteredModel = (
   projectName?: string,
   platform?: ServingRuntimePlatform,
-): Error | undefined => {
+): { loaded: boolean; error: Error | undefined } => {
   const [servingRuntimes, loaded, loadError] = useServingRuntimes(projectName);
 
   // If project is not selected, there is no error
   if (!projectName) {
-    return undefined;
+    return { loaded: true, error: undefined };
   }
 
   // If the platform is not selected
   if (!platform) {
-    return new Error('Cannot deploy the model until you select a model serving platform');
+    return {
+      loaded,
+      error: new Error('Cannot deploy the model until you select a model serving platform'),
+    };
   }
 
   if (loadError) {
-    return loadError;
+    return { loaded: true, error: loadError };
   }
 
   // If the platform is MULTI but it doesn't have a server
   if (platform === ServingRuntimePlatform.MULTI) {
     if (loaded && servingRuntimes.length === 0) {
-      return new Error('Cannot deploy the model until you configure a model server');
+      return {
+        loaded,
+        error: new Error('Cannot deploy the model until you configure a model server'),
+      };
     }
   }
 
-  return undefined;
+  return { loaded, error: undefined };
 };
 
 export default useProjectErrorForRegisteredModel;

frontend/src/pages/modelRegistry/screens/__tests__/useProjectErrorForRegisteredModel.spec.ts

@@ -47,7 +47,7 @@ describe('useProjectErrorForRegisteredModel', () => {
     const renderResult = testHook(useProjectErrorForRegisteredModel)(undefined, undefined);
     // wait for update
     await renderResult.waitForNextUpdate();
-    expect(renderResult).hookToStrictEqual(undefined);
+    expect(renderResult).hookToStrictEqual({ loaded: true, error: undefined });
   });
 
   it('should return undefined when only kServe is supported', async () => {
@@ -58,7 +58,7 @@ describe('useProjectErrorForRegisteredModel', () => {
     );
     // wait for update
     await renderResult.waitForNextUpdate();
-    expect(renderResult).hookToStrictEqual(undefined);
+    expect(renderResult).hookToStrictEqual({ loaded: true, error: undefined });
   });
 
   it('should return undefined when only modelMesh is supported with server deployed', async () => {
@@ -69,7 +69,7 @@ describe('useProjectErrorForRegisteredModel', () => {
     );
     // wait for update
     await renderResult.waitForNextUpdate();
-    expect(renderResult).hookToStrictEqual(undefined);
+    expect(renderResult).hookToStrictEqual({ loaded: true, error: undefined });
   });
 
   it('should return error when only modelMesh is supported with no server deployed', async () => {
@@ -80,18 +80,20 @@ describe('useProjectErrorForRegisteredModel', () => {
     );
     // wait for update
     await renderResult.waitForNextUpdate();
-    expect(renderResult).hookToStrictEqual(
-      new Error('Cannot deploy the model until you configure a model server'),
-    );
+    expect(renderResult).hookToStrictEqual({
+      loaded: true,
+      error: new Error('Cannot deploy the model until you configure a model server'),
+    });
   });
 
   it('should return error when platform is not selected', async () => {
     k8sListResourceMock.mockResolvedValue(mockK8sResourceList([]));
     const renderResult = testHook(useProjectErrorForRegisteredModel)('test-project', undefined);
     // wait for update
     await renderResult.waitForNextUpdate();
-    expect(renderResult).hookToStrictEqual(
-      new Error('Cannot deploy the model until you select a model serving platform'),
-    );
+    expect(renderResult).hookToStrictEqual({
+      loaded: true,
+      error: new Error('Cannot deploy the model until you select a model serving platform'),
+    });
   });
 });

frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx

@@ -37,13 +37,14 @@ const DeployRegisteredModelModal: React.FC<DeployRegisteredModelModalProps> = ({
     selectedProject,
     servingPlatformStatuses,
   );
-  const projectError = useProjectErrorForRegisteredModel(selectedProject?.metadata.name, platform);
+  const { loaded: projectDeployStatusLoaded, error: projectError } =
+    useProjectErrorForRegisteredModel(selectedProject?.metadata.name, platform);
   const [dataConnections] = useDataConnections(selectedProject?.metadata.name);
   const error = platformError || projectError;
 
   const {
     registeredModelDeployInfo,
-    loaded,
+    loaded: deployInfoLoaded,
     error: deployInfoError,
   } = useRegisteredModelDeployInfo(modelVersion);
 
@@ -52,7 +53,11 @@ const DeployRegisteredModelModal: React.FC<DeployRegisteredModelModalProps> = ({
     onCancel();
   }, [onCancel]);
 
-  if (!selectedProject || !platform) {
+  if (
+    (platform === ServingRuntimePlatform.MULTI && !projectDeployStatusLoaded) ||
+    !selectedProject ||
+    !platform
+  ) {
     return (
       <Modal
         title="Deploy model"
@@ -75,7 +80,7 @@ const DeployRegisteredModelModal: React.FC<DeployRegisteredModelModalProps> = ({
             <Alert variant="danger" isInline title={deployInfoError.name}>
               {deployInfoError.message}
             </Alert>
-          ) : !loaded ? (
+          ) : !deployInfoLoaded ? (
             <Spinner />
           ) : (
             <ProjectSelector

frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/DataConnectionExistingField.tsx

@@ -1,5 +1,14 @@
 import * as React from 'react';
-import { Button, FormGroup, Popover, Stack, StackItem } from '@patternfly/react-core';
+import {
+  Button,
+  Flex,
+  FlexItem,
+  FormGroup,
+  Label,
+  Popover,
+  Stack,
+  StackItem,
+} from '@patternfly/react-core';
 import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons';
 import { UpdateObjectAtPropAndValue } from '~/pages/projects/types';
 import {
@@ -31,6 +40,22 @@ const DataConnectionExistingField: React.FC<DataConnectionExistingFieldType> = (
   const selectedDataConnection = connectionsWithoutBucket.find(
     (connection) => connection.dataConnection.data.metadata.name === data.storage.dataConnection,
   );
+
+  const getLabeledOption = (connection: LabeledDataConnection) => (
+    <Flex
+      spaceItems={{ default: 'spaceItemsXs' }}
+      data-testid={`inference-service-data-connection ${connection.dataConnection.data.metadata.name}`}
+    >
+      <FlexItem>{getDataConnectionDisplayName(connection.dataConnection)}</FlexItem>
+      {connection.isRecommended && (
+        <FlexItem>
+          <Label color="blue" isCompact>
+            Recommended
+          </Label>
+        </FlexItem>
+      )}
+    </Flex>
+  );
   return (
     <Stack hasGutter>
       <StackItem>
@@ -56,13 +81,12 @@ const DataConnectionExistingField: React.FC<DataConnectionExistingFieldType> = (
             isDisabled={isDataConnectionsEmpty}
             options={connectionsWithoutBucket.map((connection) => ({
               key: connection.dataConnection.data.metadata.name,
+              dropdownLabel: getLabeledOption(connection),
               label: getDataConnectionDisplayName(connection.dataConnection),
             }))}
             value={data.storage.dataConnection}
             toggleLabel={
-              selectedDataConnection
-                ? getDataConnectionDisplayName(selectedDataConnection.dataConnection)
-                : placeholderText
+              selectedDataConnection ? getLabeledOption(selectedDataConnection) : placeholderText
             }
             onChange={(option) => {
               setData('storage', {