Skip to content

Serverless create, build and run functions in OpenShift#3003

Merged
mohitsuman merged 24 commits intoredhat-developer:mainfrom
msivasubramaniaan:2993-implement-serverless-functions-into-openshift-toolkit-extension
Jul 24, 2023
Merged

Serverless create, build and run functions in OpenShift#3003
mohitsuman merged 24 commits intoredhat-developer:mainfrom
msivasubramaniaan:2993-implement-serverless-functions-into-openshift-toolkit-extension

Conversation

@msivasubramaniaan
Copy link
Copy Markdown
Collaborator

@msivasubramaniaan msivasubramaniaan commented Jul 3, 2023

Fixes: #2993

Currently This PR covers the Create, Build and Run functionalities of func cli and the Deploy, invoke function will be covered by #3010

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@msivasubramaniaan msivasubramaniaan self-assigned this Jul 3, 2023
@msivasubramaniaan msivasubramaniaan marked this pull request as draft July 3, 2023 18:42
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 10, 2023

Codecov Report

Patch coverage: 24.77% and project coverage change: -1.00 ⚠️

Comparison is base (b41b3da) 35.34% compared to head (fc02ab4) 34.35%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3003      +/-   ##
==========================================
- Coverage   35.34%   34.35%   -1.00%     
==========================================
  Files          64       71       +7     
  Lines        4255     4687     +432     
  Branches      840      909      +69     
==========================================
+ Hits         1504     1610     +106     
- Misses       2751     3077     +326     
Impacted Files Coverage Δ
src/k8s/deploymentConfig.ts 94.80% <0.00%> (-2.53%) ⬇️
src/openshift/cluster.ts 10.42% <0.00%> (-0.11%) ⬇️
src/openshift/project.ts 41.81% <0.00%> (-0.78%) ⬇️
src/serveressFunction/build-run-deploy.ts 5.51% <5.51%> (ø)
src/webview/git-import/gitImportLoader.ts 16.66% <20.00%> (+0.20%) ⬆️
...ew/serverless-function/serverlessFunctionLoader.ts 20.31% <20.31%> (ø)
src/serveressFunction/commands.ts 25.00% <25.00%> (ø)
src/util/workspace.ts 23.40% <25.00%> (-2.92%) ⬇️
src/serveressFunction/functionImpl.ts 32.07% <32.07%> (ø)
src/webview/common/utils.ts 33.33% <33.33%> (ø)
... and 5 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@msivasubramaniaan msivasubramaniaan marked this pull request as ready for review July 10, 2023 16:17
@msivasubramaniaan msivasubramaniaan changed the title [WIP] Serverless functions in OpenShift Serverless create, build and run functions in OpenShift Jul 10, 2023
@datho7561
Copy link
Copy Markdown
Contributor

I'm deleting my previous comments to make my feedback more concise, and since some of the things I said were because I didn't know what I was doing.

After playing around with the feature a bit more, I figured out the following things, which helped me get through the "Create", "Build", and "Run" steps of the UI:

  • You need to be signed into a image registry, such as quay.io or dockerhub, in order for this UI to work
  • You need to set up docker-credential-pass with your image registry username and password, since kn/func cannot use the fact that you are logged in with docker/podman to access the registry
  • The image is not pushed to the image registry during the build step, it's built locally and tagged with an appropriate name
  • The image is not pulled from the specified image registry during the "Run", it uses the local build of the image. By default, the image is run using podman/docker and is accessible through localhost.

Things we can improve:

  • We need to provide an error popup if the build fails. There was no indication that the build failed and I was confused why the wizard wouldn't advance to step 3 after the build finished.
  • If I load a project folder for an existing serverless function, it won't appear in the tree view. It should be fairly simple to detect if a folder is a serverless function folder, since it has a func.yaml file at the root of the workspace
  • If I create a new serverless function, then the source folder isn't added to the current VS Code workspace until I close the wizard. I think it should be added right after you click "Create" in the first step of the wizard.
  • If I close the wizard before I finish creating a new serverless function, then it attempts to add the folder /new to the workspace. We shouldn't add any folders to the workspace if the user didn't finish filling out the first step of the wizard.
  • After the serverless function is set up, it should be possible to build and run the function without opening the Webview. i.e. the context menu items "Build" and "Run" should perform the operation without opening the Webview.

@msivasubramaniaan
Copy link
Copy Markdown
Collaborator Author

msivasubramaniaan commented Jul 13, 2023

I'm deleting my previous comments to make my feedback more concise, and since some of the things I said were because I didn't know what I was doing.

After playing around with the feature a bit more, I figured out the following things, which helped me get through the "Create", "Build", and "Run" steps of the UI:

  • You need to be signed into a image registry, such as quay.io or dockerhub, in order for this UI to work
  • You need to set up docker-credential-pass with your image registry username and password, since kn/func cannot use the fact that you are logged in with docker/podman to access the registry
  • The image is not pushed to the image registry during the build step, it's built locally and tagged with an appropriate name
  • The image is not pulled from the specified image registry during the "Run", it uses the local build of the image. By default, the image is run using podman/docker and is accessible through localhost.

Things we can improve:

  • We need to provide an error popup if the build fails. There was no indication that the build failed and I was confused why the wizard wouldn't advance to step 3 after the build finished. - Addressed and I will push the code
  • If I load a project folder for an existing serverless function, it won't appear in the tree view. It should be fairly simple to detect if a folder is a serverless function folder, since it has a func.yaml file at the root of the workspace - Tested in my local and it is working fine on both ways 1) Open Folder 2) Add to workspace. Let me know how you tested?
  • If I create a new serverless function, then the source folder isn't added to the current VS Code workspace until I close the wizard. I think it should be added right after you click "Create" in the first step of the wizard. - If we add after create then the extension was restarts, we lost the WebView control, So provided "Finish" option on each level, if user wants to finish then it will add it to workspace, or if user closed it does.
  • If I close the wizard before I finish creating a new serverless function, then it attempts to add the folder /new to the workspace. We shouldn't add any folders to the workspace if the user didn't finish filling out the first step of the wizard. - Will work on this
  • After the serverless function is set up, it should be possible to build and run the function without opening the Webview. i.e. the context menu items "Build" and "Run" should perform the operation without opening the Webview. - If so we need to use vscode open dialog for getting build name and other required details, To overcome the vscode dialogs, open it on webview. So the flow was uniform across

@datho7561
Copy link
Copy Markdown
Contributor

Tested in my local and it is working fine on both ways 1) Open Folder 2) Add to workspace. Let me know how you tested?

I can't reproduce this issue, my bad

If so we need to use vscode open dialog for getting build name and other required details, To overcome the vscode dialogs, open it on webview. So the flow was uniform across

You can run func build and func run on the command line without passing these parameters. I think that you only need to specify the parameters the first time that you run the Knative function.

@msivasubramaniaan
Copy link
Copy Markdown
Collaborator Author

Tested in my local and it is working fine on both ways 1) Open Folder 2) Add to workspace. Let me know how you tested?

I can't reproduce this issue, my bad - Np

If so we need to use vscode open dialog for getting build name and other required details, To overcome the vscode dialogs, open it on webview. So the flow was uniform across

You can run func build and func run on the command line without passing these parameters. I think that you only need to specify the parameters the first time that you run the Knative function. - It is not only on the first time, user can update the build image if needed while build, Also there was an option run, user can choose either run with build/not.

Regarding the error dialog and empty workspace added I will push the code. Please have a look

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@datho7561
Copy link
Copy Markdown
Contributor

Regarding the error dialog

"Build" doesn't seem to be working any more, there is no output in the terminal. It works from the command line (func build) but not in the UI.

and empty workspace added I will push the code. Please have a look

Okay, the issue where it adds /new to the workspace if I close the wizard before filling it out is gone.

It is not only on the first time, user can update the build image if needed while build

I don't think that changing the name of the container image is that common, since if other people are pulling the image from a container registry, they expect the name of the container image to remain the same. I think that func keeps track of the name of the built image under the image field in the func.yaml file. What is your opinion on setting this value in the first step of the wizard, then providing an "Edit" wizard where the user can edit these values?

Also there was an option run,

What do think about having a context menu item "Build and Run"? So we could have these context items:

  • Build
  • Build and Run
  • Run without Rebuilding

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: unknown <msivasub@win.redhat.com>
datho7561
datho7561 previously approved these changes Jul 20, 2023
Copy link
Copy Markdown
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks, Muthu!

Signed-off-by: unknown <msivasub@win.redhat.com>
Comment thread package.json
Comment thread src/tools.json
Comment thread src/util/workspace.ts Outdated
Comment thread src/webview/serverless-function/app/createFunction.tsx Outdated
Comment thread src/webview/serverless-function/app/createFunction.tsx Outdated
Comment thread src/webview/serverless-function/app/createFunction.tsx Outdated
Comment thread src/webview/serverless-function/app/createFunction.tsx Outdated
Comment thread src/webview/serverless-function/app/createFunction.tsx Outdated
Comment thread src/webview/serverless-function/app/home.tsx Outdated
Copy link
Copy Markdown
Contributor

@mohitsuman mohitsuman left a comment

Choose a reason for hiding this comment

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

Reviewed the wordings and UX changes.

unknown added 2 commits July 24, 2023 16:14
Signed-off-by: unknown <msivasub@win.redhat.com>
Signed-off-by: unknown <msivasub@win.redhat.com>
Copy link
Copy Markdown
Contributor

@mohitsuman mohitsuman left a comment

Choose a reason for hiding this comment

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

lgtm

@mohitsuman mohitsuman merged commit 3226372 into redhat-developer:main Jul 24, 2023
@datho7561
Copy link
Copy Markdown
Contributor

  • Clicking the "Language" option in the drop down causes the page to go blank, so I can't create a new function
  • In my opinion, the UI elements, in particular the text field labels, no longer look like Material (see material design text field)

@datho7561
Copy link
Copy Markdown
Contributor

oh

@msivasubramaniaan
Copy link
Copy Markdown
Collaborator Author

  • Clicking the "Language" option in the drop down causes the page to go blank, so I can't create a new function
  • In my opinion, the UI elements, in particular the text field labels, no longer look like Material (see material design text field)

#3049 will fix the mentioned issue

@datho7561 datho7561 mentioned this pull request Jul 24, 2023
@msivasubramaniaan msivasubramaniaan deleted the 2993-implement-serverless-functions-into-openshift-toolkit-extension branch September 8, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Serverless Functions into OpenShift Toolkit extension

3 participants