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

[WIP] Angular migration #2727

Closed
wants to merge 51 commits into from
Closed

[WIP] Angular migration #2727

wants to merge 51 commits into from

Conversation

maciaszczykm
Copy link
Member

@maciaszczykm maciaszczykm commented Jan 3, 2018

Progress

  • create page (to be done by @pengx17)
  • logs (to be done by @yinhaijiao)
  • exec into pod
  • custom with exec and logs actionbar for pods
  • usage graphs for all views (to be done by @austbot)
  • tests (to be done by @austbot)
  • TODOs
  • check if nothing is missing - same functionality as in AngularJS

Test deployment

https://gist.github.com/floreks/7bafe96d835237ae645a2a5f38872581

Deploy command: kubectl apply -f https://gist.githubusercontent.com/floreks/7bafe96d835237ae645a2a5f38872581/raw/7040b129597bc5d5720b781e5a5da1f2714cdf81/kubernetes-dashboard-v2.0.0-alpha0.yaml

Access it using kubectl proxy at: http://localhost:8001/api/v1/namespaces/kube-system/services/https:kubernetes-dashboard-v2:/proxy/

To grant cluster admin privileges for tests use: kubectl apply -f https://gist.githubusercontent.com/floreks/d1bff5386b07b4e3690eed2fb136c94f/raw/2d59ebd9e58ad3f748bb1de1ecd3f5ef07d46531/kubernetes-dashboard-v2.0.0-alpha0-admin-binding.yaml

Sneak peek of current changes

image
image
image
zrzut ekranu z 2018-02-21 11-15-03
zrzut ekranu z 2018-02-21 11-15-29
zrzut ekranu z 2018-03-05 16-14-17

@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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 3, 2018
@maciaszczykm maciaszczykm added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Jan 4, 2018
@floreks floreks force-pushed the angular-migration branch from 4f7cef8 to ccdf939 Compare January 4, 2018 09:34
@pengx17
Copy link
Contributor

pengx17 commented Jan 10, 2018

I am wondering how to contribute to this Angular migration branch?

@maciaszczykm
Copy link
Member Author

@pengx17 In which part are you willing to help? You can use pull request for it, but we have to coordinate :)

@pengx17
Copy link
Contributor

pengx17 commented Jan 10, 2018

@maciaszczykm I suppose in some later stage of the migration once the new infrastructure for Angular is up, there will be a second stage that migrating existing features and common utility services/components/directives.
I guess it is hard to have the community take participate into the infra build up, thus I am willing to take into the second part.

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 11, 2018
@floreks floreks force-pushed the angular-migration branch 18 times, most recently from 28c88f9 to a7084ed Compare January 12, 2018 14:13
@RicardoVaranda
Copy link

Hi guys, great work you have done so far, I'm wondering if there is any possibility of me helping you out and contributing to this PR.

@yinhaijiao
Copy link
Contributor

@maciaszczykm Excuse me. Is the migration branche in progress now?

@maciaszczykm
Copy link
Member Author

@yinhaijiao Unfortunately not for a while.

@yinhaijiao
Copy link
Contributor

@maciaszczykm Will it continue in the future?

@maciaszczykm
Copy link
Member Author

@yinhaijiao I don't know. It looks unoptimistic at the moment.

@timn
Copy link

timn commented May 13, 2018

@maciaszczykm, what is the reason for your "unoptimism"? I have actually used part of the code in another project. It worked quite nicely.

@floreks
Copy link
Member

floreks commented May 13, 2018

@timn We have been moved to another project and don't really have time to contribute anymore. That is why the future of this migration does not look optimistic.

@danielromlein
Copy link
Contributor

@konryd do you know why the screenshots here are dark and orange and not the original Dashboard colors?

@timn
Copy link

timn commented Jun 5, 2018

The new version features two color themes you can choose from, a light and a dark one. The original (light) is still there as far as I can tell from the source, cf. https://github.com/kubernetes/dashboard/blob/angular-migration/src/app/frontend/index.scss#L16 (see the imported light and dark theme style files).

@maciaszczykm
Copy link
Member Author

@timn is right. We were rewriting app from the scrach, so it was quite easy to introduce second theme and and few other things. You can check current progress by running the code directly from the branch or just using test deployment (I do not remember if it is 100% up to date with this branch).

Colors of both themes can be easily changed, as there is one CSS configuration file per theme.

@danielromlein
Copy link
Contributor

@timn @maciaszczykm cool! We should get some Design input there to ensure a11y, brand alignment, etc. 👍

@mgred
Copy link
Contributor

mgred commented Jun 19, 2018

Hey, is there still some help wanted for the migration, or is the project frozen so far?

@danielromlein
Copy link
Contributor

@mgred: @bryk could better definitively weigh in here, but I believe that because of the massive amount of effort it will require, the Angular migration is on hold in favor of other open issues. Definitely want to find a place for you to jump in and contribute if you're interested!

@mgred
Copy link
Contributor

mgred commented Jun 21, 2018

@danielromlein, @bryk Let me know if there is any task to contribute to or review the current migration, I'am definitely willing to help out here.

@konryd
Copy link

konryd commented Jul 2, 2018

@mgred Hey Marc, I'm covering for @bryk who is on sabbatical.

I believe this PR should be abandoned in its current form. It's a stop-the-world migration which was carried mid-way and hasn't seen large changes since March. In the meantime, the upstream angular1 version has accumulated new features and bugfixes. Merging at this point poses too large a risk.

Having said that: migrating to ng2 is still desireable, but we need to find a gradual approach that lets us migrate one page after another. Specifically: we need a way of serving both ng1 and ng2 pages from the same installation. Solving this problem will both unlock the migration itself and let multiple contributors to work on it in parallel. Ping me on slack for more details.

@floreks
Copy link
Member

floreks commented Jul 2, 2018

Having said that: migrating to ng2 is still desireable, but we need to find a gradual approach that lets us migrate one page after another. Specifically: we need a way of serving both ng1 and ng2 pages from the same installation. Solving this problem will both unlock the migration itself and let multiple contributors to work on it in parallel. Ping me on slack for more details.

Before starting the migration process we were looking at more flexible approaches. Unfortunately, the build pipeline for AngularJS in its current form doesn't really allow us to work with ng1 and ng2 at the same time. Also, the effort to use half-way solutions and the need to rewrite everything to the "pure" Angular 6 after finishing rewriting the AngularJS pages to Angular was a no-go for us.

We have spent quite some time investigating how to proceed with the migration and actually starting with the fresh build pipeline was the best and the cleanest solution.

I'd say that at the moment migration is finished in ~60-70%. The core is there, critical parts of the application were rewritten and it is possible to rewrite 1 view at the time.

I'd only suggest to split it a bit and start with a separate branch where we could keep the migration process going in parallel to the AngularJS version. This would allow us to introduce smaller PRs related to the migration. In the end, when the migration branch will be in sync with the master we can officially release the new version.

@maciaszczykm
Copy link
Member Author

In the meantime, the upstream angular1 version has accumulated new features and bugfixes. Merging at this point poses too large a risk.

@konryd It is true, that angular-migration branch is a bit behind master (~50 commits), but since we have stopped contributing most of the commits are typo fixes, translations, greenkeeper updates etc. I do not see any new features besides applications lists (it is not merged yet) that can cause problems. See yourself: https://github.com/kubernetes/dashboard/commits/master

I think, that I can take care of merge/rebase in the coming days.

As @floreks said, we have already spend a lot of time investigating possible options, for us rewriting frontend from the scratch was the best solution. The build process, translations mechanism that is implemented and few others things pushed us towards it. I agree that "stop-the-world migration" is not perfect, but I believe this is the way we should proceed as we are really close to the finish. We have put a lot of effort here and I believe we can finish it a lot quicker and easier than starting from the beginnig, where setup of build pipeline can take similar time, not mentioning whole migration.

@pengx17
Copy link
Contributor

pengx17 commented Jul 2, 2018

I agreed to @maciaszczykm. The main branch only involves some minor fixes/features and perhaps will have Application List as the last major feature. It might take a lot of efforts to make a parallel app with both versions of Angular running but the remaining features to be migrated are not too much to be done. Thus the significance of making a parallel app is questioned.

Engined with the latest tech is the most importance thing to bring this project alive than bringing all features to it. The rarely used features but which would also a lot of work to do (eg., "creating an Deployment with full feature forms") could be planned later.

In my opinion we need to think about what are the most used daily usage of the Dashboard and prioritize the todo items, so we could get the Angular branch released asap and make the contributors energetic again.

@konryd
Copy link

konryd commented Jul 4, 2018

@floreks @maciaszczykm @pengx17 Your arguments are very convincing.

Still, having done 70% of work, the remaining 70% needs to happen too :) I saw many people in the chat room asking about this, which gives me hope, but it needs someone who is well versed with the codebase to distribute the tasks and review the code (especially critical in absence of a gradual solution). Is there such a person?

@maciaszczykm
Copy link
Member Author

@konryd I think both me and @floreks will work on it again, at least to finish the migration :)

@konryd
Copy link

konryd commented Jul 4, 2018

Oh wow. This is amazing and indeed makes the problem go away :) Thanks!

@maciaszczykm
Copy link
Member Author

maciaszczykm commented Jul 4, 2018

package-lock.json should be regenerated to work with newest Node.js version:

rm -rf node_modules package-lock.json && npm i

Leaving it here as a reminder.

@austbot
Copy link
Contributor

austbot commented Jul 10, 2018

@maciaszczykm I will work on usage graphs and tests. please assign me

@maciaszczykm maciaszczykm mentioned this pull request Jul 17, 2018
55 tasks
@ysaakpr
Copy link

ysaakpr commented Sep 5, 2018

Can we consider of custom colour theming option in order to provide users to configure the different consoles to looks different, at least with respect to the color. We have multiple clusters and running multiple instances of dashboards from each cluster and while switching its easier to identify the console in which one working by just matching the color.

It will be really helpful if we can choose the primary colours from a given palette.

@maciaszczykm
Copy link
Member Author

@ysaakpr We have introduced theming functionality. At the moment there are only two themes (dark and light), but it is easy to extend. It is not our priority though, not now at least. You should take a look if you can implement new themes and add them to the upstream.

@ysaakpr
Copy link

ysaakpr commented Sep 5, 2018 via email

@floreks floreks deleted the angular-migration branch December 21, 2018 10:14
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: no Indicates the PR's author has not signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. 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.