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 plugin support on the frontend #4094

Merged
merged 23 commits into from
Sep 2, 2019

Conversation

ajatprabha
Copy link
Contributor

@ajatprabha ajatprabha commented Jul 19, 2019

Ref: #4069

  • List registered plugins in a ResourceList based component
  • Add a link to all plugins in the Sidebar
  • Add a route for plugin UI to render
  • Render the AOT compiled plugin at runtime.
  • Error Handling
  • Fetch plugins based on the namespace

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 19, 2019
@k8s-ci-robot k8s-ci-robot requested review from ianlewis and jeefy July 19, 2019 09:06
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 22, 2019
@ajatprabha
Copy link
Contributor Author

Current progress:

  1. List of installed plugins

Screenshot from 2019-07-22 17-04-19

  1. A sample plugin displaying available pods in the cluster

Screenshot from 2019-07-22 17-03-43

@maciaszczykm
Copy link
Member

Testing it :)

@maciaszczykm
Copy link
Member

The plugin works, that's great.

Few remarks from my side:

  • If there is no Plugin CRD registered in the cluster I'd avoid listing Plugins in the menu. Thanks to that users can use dashboard like they do now if the have no plugins. Otherwise, when they enter plugin page they will see errors:

Zrzut ekranu 2019-07-25 o 11 34 45

Another thing to consider here might be adding CRD from our backend and listing Plugins in the menu only if there are any plugins in the cluster.

  • I'd like to know more how the plugins can be created. Which parts of the backend I can use, which parts of the frontend I can use. How to build the plugins. It will require some detailed documentation.

  • Can you add plugin that uses material design card and styling is similar to the rest of the app? It would be valuable to show it to the people.

  • I'd really appreciate possibility to pin some of the plugins to the menu like we advised for CRDs. I mean in this demo plugin1 could be pinned to the menu and then appear just under Plugins. Like Pods appear under Workloads.

At the moment that's it. I really appreciate all the effort and I think most crucial part is already done. Keep it up!

@floreks
Copy link
Member

floreks commented Jul 25, 2019

I have found a couple of issues while testing:

  • Not working with secure Dashboard (login enabled)
    Zrzut ekranu z 2019-07-25 12-19-29
  • Delete and edit is available on the plugin list but it is not working
    Zrzut ekranu z 2019-07-25 12-24-36
  • Sorting not applied on the plugin list when first entering the page, only after manually hitting sort or clicking again on Plugins entry.

Questions:

  1. How dependencies are calculated?
  2. Are we able to inject plugin view into any view?

Besides that, really nice progress. It looks better and better.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 1, 2019
@ajatprabha ajatprabha force-pushed the plugin/frontend branch 2 times, most recently from c1abe24 to f6b49df Compare August 5, 2019 12:41
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 5, 2019
@ajatprabha ajatprabha force-pushed the plugin/frontend branch 2 times, most recently from a41c80e to aef227c Compare August 8, 2019 19:32
@codecov
Copy link

codecov bot commented Aug 8, 2019

Codecov Report

Merging #4094 into master will increase coverage by 0.56%.
The diff coverage is 45.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4094      +/-   ##
==========================================
+ Coverage   46.01%   46.57%   +0.56%     
==========================================
  Files         198      200       +2     
  Lines        9300     9366      +66     
  Branches      105      105              
==========================================
+ Hits         4279     4362      +83     
+ Misses       4760     4735      -25     
- Partials      261      269       +8
Impacted Files Coverage Δ
src/app/backend/api/types.go 38.88% <ø> (ø) ⬆️
src/app/backend/handler/apihandler.go 27.81% <0%> (+0.57%) ⬆️
...rontend/common/components/resourcelist/groupids.ts 100% <100%> (ø) ⬆️
src/app/backend/plugin/handler.go 78.94% <100%> (+53.3%) ⬆️
.../app/frontend/common/services/resource/endpoint.ts 79.24% <100%> (+0.39%) ⬆️
src/app/frontend/login/component.ts 79.16% <100%> (+0.9%) ⬆️
src/app/backend/plugin/config.go 48.64% <48.64%> (ø)
src/app/frontend/common/services/global/plugin.ts 50% <50%> (ø)
src/app/backend/client/verber.go 66.26% <50%> (-1.64%) ⬇️
src/app/backend/client/manager.go 64.37% <71.42%> (+1.82%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f647ff...880c92e. Read the comment docs.

@ajatprabha ajatprabha changed the title [WIP] Add plugin support on the frontend Add plugin support on the frontend Aug 26, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 26, 2019
@floreks
Copy link
Member

floreks commented Sep 2, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 2, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ajatprabha, floreks

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 2, 2019
@k8s-ci-robot k8s-ci-robot merged commit 90de38f into kubernetes:master Sep 2, 2019
@ajatprabha ajatprabha mentioned this pull request Nov 21, 2019
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants