Improve update call in editing workbench

backend/src/routes/api/k8s/index.ts

@@ -40,6 +40,7 @@ module.exports = async (fastify: KubeFastifyInstance) => {
         url,
         method: req.method,
         requestData: data,
+        overrideContentType: req.headers['content-type'],
       };
 
       let promise: Promise<unknown>;

backend/src/routes/api/k8s/pass-through.ts

@@ -12,6 +12,8 @@ export type PassThroughData = {
   method: string;
   requestData?: string;
   url: string;
+  /** Option to substitute your own content type for the API call -- defaults to JSON */
+  overrideContentType?: string;
 };
 
 export const isK8sStatus = (data: unknown): data is K8sStatus =>

backend/src/server.ts

@@ -33,6 +33,21 @@ const app = fastify({
   pluginTimeout: 10000,
 });
 
+// Allow the fastify server to parse the merge-patch/json content type
+app.addContentTypeParser(
+  'application/merge-patch+json',
+  { parseAs: 'string' },
+  function (req, body, done) {
+    try {
+      const json = JSON.parse(String(body));
+      done(null, json);
+    } catch (err) {
+      err.statusCode = 400;
+      done(err, undefined);
+    }
+  },
+);
+
 app.register(initializeApp);
 
 app.listen({ port: PORT, host: IP }, (err) => {

frontend/src/__mocks__/mockStartNotebookData.ts

@@ -4,12 +4,18 @@ import { mockK8sNameDescriptionFieldData } from '~/__mocks__/mockK8sNameDescript
 
 type MockResourceConfigType = {
   volumeName?: string;
+  notebookId?: string;
 };
 export const mockStartNotebookData = ({
+  notebookId,
   volumeName = 'test-volume',
 }: MockResourceConfigType): StartNotebookData => ({
   projectName: 'test-project',
-  notebookData: mockK8sNameDescriptionFieldData({ name: 'test-notebook', description: '' }),
+  notebookData: mockK8sNameDescriptionFieldData({
+    name: 'test-notebook',
+    description: '',
+    k8sName: { value: notebookId },
+  }),
   image: {
     imageStream: {
       metadata: {

frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts

@@ -556,9 +556,10 @@ describe('Workbench page', () => {
     editSpawnerPage.findSubmitButton().should('be.enabled');
     editSpawnerPage.k8sNameDescription.findDisplayNameInput().fill('Updated Notebook');
 
-    cy.interceptK8s('PUT', NotebookModel, mockNotebookK8sResource({})).as('editWorkbench');
+    cy.interceptK8s('PUT', NotebookModel, mockNotebookK8sResource({})).as('editWorkbenchDryRun');
+    cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('editWorkbench');
     editSpawnerPage.findSubmitButton().click();
-    cy.wait('@editWorkbench').then((interception) => {
+    cy.wait('@editWorkbenchDryRun').then((interception) => {
       expect(interception.request.url).to.include('?dryRun=All');
       expect(interception.request.body).to.containSubset({
         metadata: {
@@ -580,15 +581,10 @@ describe('Workbench page', () => {
         },
       });
     });
-
     // Actual request
     cy.wait('@editWorkbench').then((interception) => {
       expect(interception.request.url).not.to.include('?dryRun=All');
     });
-
-    cy.get('@editWorkbench.all').then((interceptions) => {
-      expect(interceptions).to.have.length(2); // 1 dry run request and 1 actual request
-    });
   });
 
   it('Handle deleted notebook sizes in workbenches table', () => {

frontend/src/api/k8s/__tests__/notebooks.spec.ts

@@ -3,11 +3,9 @@ import {
   k8sGetResource,
   k8sPatchResource,
   k8sListResource,
-  k8sUpdateResource,
   k8sDeleteResource,
   K8sStatus,
 } from '@openshift/dynamic-plugin-sdk-utils';
-import * as _ from 'lodash-es';
 import { NotebookKind } from '~/k8sTypes';
 import { Toleration } from '~/types';
 import { mockNotebookK8sResource } from '~/__mocks__/mockNotebookK8sResource';
@@ -23,7 +21,6 @@ import {
   getNotebooks,
   stopNotebook,
   startNotebook,
-  updateNotebook,
   deleteNotebook,
   attachNotebookSecret,
   replaceNotebookSecret,
@@ -32,6 +29,7 @@ import {
   removeNotebookSecret,
   getStopPatch,
   startPatch,
+  mergePatchUpdateNotebook,
 } from '~/api/k8s/notebooks';
 
 import {
@@ -43,14 +41,18 @@ import {
 
 import { TolerationChanges, getTolerationPatch } from '~/utilities/tolerations';
 import { NotebookModel } from '~/api/models';
+import { k8sMergePatchResource } from '~/api/k8sUtils';
 
 jest.mock('@openshift/dynamic-plugin-sdk-utils', () => ({
   k8sCreateResource: jest.fn(),
   k8sDeleteResource: jest.fn(),
   k8sGetResource: jest.fn(),
   k8sListResource: jest.fn(),
   k8sPatchResource: jest.fn(),
-  k8sUpdateResource: jest.fn(),
+}));
+
+jest.mock('~/api/k8sUtils', () => ({
+  k8sMergePatchResource: jest.fn(),
 }));
 
 jest.mock('~/concepts/pipelines/elyra/utils', () => {
@@ -65,7 +67,7 @@ const k8sCreateResourceMock = jest.mocked(k8sCreateResource<NotebookKind>);
 const k8sGetResourceMock = jest.mocked(k8sGetResource<NotebookKind>);
 const k8sPatchResourceMock = jest.mocked(k8sPatchResource<NotebookKind>);
 const k8sListResourceMock = jest.mocked(k8sListResource<NotebookKind>);
-const k8sUpdateResourceMock = jest.mocked(k8sUpdateResource<NotebookKind>);
+const k8sMergePatchResourceMock = jest.mocked(k8sMergePatchResource<NotebookKind>);
 const k8sDeleteResourceMock = jest.mocked(k8sDeleteResource<NotebookKind, K8sStatus>);
 const createElyraServiceAccountRoleBindingMock = jest.mocked(createElyraServiceAccountRoleBinding);
 
@@ -468,66 +470,52 @@ describe('stopNotebook', () => {
 });
 describe('updateNotebook', () => {
   it('should update a notebook', async () => {
-    const notebook = assembleNotebook(mockStartNotebookData({}), username);
-
     const existingNotebook = mockNotebookK8sResource({ uid });
-    const oldNotebook = structuredClone(existingNotebook);
-    const container = oldNotebook.spec.template.spec.containers[0];
-
-    // clean the envFrom array in case of merging the old value again
-    container.envFrom = [];
-
-    // clean the resources, affinity and tolerations for accelerator
-    oldNotebook.spec.template.spec.tolerations = [];
-    oldNotebook.spec.template.spec.affinity = {};
-    container.resources = {};
+    const notebook = assembleNotebook(
+      mockStartNotebookData({ notebookId: existingNotebook.metadata.name }),
+      username,
+    );
 
-    k8sUpdateResourceMock.mockResolvedValue(existingNotebook);
+    k8sMergePatchResourceMock.mockResolvedValue(existingNotebook);
 
-    const renderResult = await updateNotebook(
-      existingNotebook,
-      mockStartNotebookData({}),
+    const renderResult = await mergePatchUpdateNotebook(
+      mockStartNotebookData({ notebookId: existingNotebook.metadata.name }),
       username,
     );
 
-    expect(k8sUpdateResourceMock).toHaveBeenCalledWith({
+    expect(k8sMergePatchResourceMock).toHaveBeenCalledWith({
       fetchOptions: {
         requestInit: {},
       },
       model: NotebookModel,
       queryOptions: { queryParams: {} },
-      resource: _.merge({}, oldNotebook, notebook),
+      resource: notebook,
     });
-    expect(k8sUpdateResourceMock).toHaveBeenCalledTimes(1);
+    expect(k8sMergePatchResourceMock).toHaveBeenCalledTimes(1);
     expect(renderResult).toStrictEqual(existingNotebook);
   });
   it('should handle errors and rethrow', async () => {
-    const notebook = assembleNotebook(mockStartNotebookData({}), username);
     const existingNotebook = mockNotebookK8sResource({ uid });
+    const notebook = assembleNotebook(
+      mockStartNotebookData({ notebookId: existingNotebook.metadata.name }),
+      username,
+    );
 
-    const oldNotebook = structuredClone(existingNotebook);
-    const container = oldNotebook.spec.template.spec.containers[0];
-
-    // clean the envFrom array in case of merging the old value again
-    container.envFrom = [];
-
-    // clean the resources, affinity and tolerations for accelerator
-    oldNotebook.spec.template.spec.tolerations = [];
-    oldNotebook.spec.template.spec.affinity = {};
-    container.resources = {};
-
-    k8sUpdateResourceMock.mockRejectedValue(new Error('error1'));
+    k8sMergePatchResourceMock.mockRejectedValue(new Error('error1'));
     await expect(
-      updateNotebook(existingNotebook, mockStartNotebookData({}), username),
+      mergePatchUpdateNotebook(
+        mockStartNotebookData({ notebookId: existingNotebook.metadata.name }),
+        username,
+      ),
     ).rejects.toThrow('error1');
-    expect(k8sUpdateResourceMock).toHaveBeenCalledTimes(1);
-    expect(k8sUpdateResourceMock).toHaveBeenCalledWith({
+    expect(k8sMergePatchResourceMock).toHaveBeenCalledTimes(1);
+    expect(k8sMergePatchResourceMock).toHaveBeenCalledWith({
       fetchOptions: {
         requestInit: {},
       },
       model: NotebookModel,
       queryOptions: { queryParams: {} },
-      resource: _.merge({}, oldNotebook, notebook),
+      resource: notebook,
     });
   });
 });

frontend/src/api/k8s/notebooks.ts

@@ -4,9 +4,9 @@ import {
   k8sGetResource,
   k8sListResource,
   k8sPatchResource,
-  k8sUpdateResource,
   Patch,
   K8sStatus,
+  k8sUpdateResource,
 } from '@openshift/dynamic-plugin-sdk-utils';
 import * as _ from 'lodash-es';
 import { NotebookModel } from '~/api/models';
@@ -26,6 +26,7 @@ import {
 } from '~/concepts/pipelines/elyra/utils';
 import { Volume, VolumeMount } from '~/types';
 import { getImageStreamDisplayName } from '~/pages/projects/screens/spawner/spawnerUtils';
+import { k8sMergePatchResource } from '~/api/k8sUtils';
 import { assemblePodSpecOptions, getshmVolume, getshmVolumeMount } from './utils';
 
 export const assembleNotebook = (
@@ -301,6 +302,21 @@ export const updateNotebook = (
   );
 };
 
+export const mergePatchUpdateNotebook = (
+  assignableData: StartNotebookData,
+  username: string,
+  opts?: K8sAPIOptions,
+): Promise<NotebookKind> =>
+  k8sMergePatchResource<NotebookKind>(
+    applyK8sAPIOptions(
+      {
+        model: NotebookModel,
+        resource: assembleNotebook(assignableData, username),
+      },
+      opts,
+    ),
+  );
+
 export const deleteNotebook = (notebookName: string, namespace: string): Promise<K8sStatus> =>
   k8sDeleteResource<NotebookKind, K8sStatus>({
     model: NotebookModel,

frontend/src/api/k8sUtils.ts

@@ -1,7 +1,11 @@
+import { applyOverrides } from '@openshift/dynamic-plugin-sdk';
 import {
+  commonFetchJSON,
+  getK8sResourceURL,
   K8sGroupVersionKind,
   K8sModelCommon,
   K8sResourceCommon,
+  K8sResourceUpdateOptions,
 } from '@openshift/dynamic-plugin-sdk-utils';
 
 export const addOwnerReference = <R extends K8sResourceCommon>(
@@ -40,3 +44,30 @@
   version: model.apiVersion,
   kind: model.kind,
 });
+
+export const k8sMergePatchResource = <
+  TResource extends K8sResourceCommon,
+  TUpdatedResource extends TResource = TResource,
+>({
+  model,
+  resource,
+  queryOptions = {},
+  fetchOptions = {},
+}: K8sResourceUpdateOptions<TResource>): Promise<TUpdatedResource> => {
+  if (!resource.metadata?.name) {
+    return Promise.reject(new Error('Resource payload name not specified'));
+  }
+
+  return commonFetchJSON<TUpdatedResource>(
+    getK8sResourceURL(model, resource, queryOptions),
+    applyOverrides(fetchOptions.requestInit, {
+      method: 'PATCH',
+      headers: {
+        'Content-Type': 'application/merge-patch+json',
+      },
+      body: JSON.stringify(resource),
+    }),
+    fetchOptions.timeout,
+    true,
+  );
+};