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

Prepare to integrate new scheduler into apache/openwhisk-deploy-kube #5278

Merged
merged 10 commits into from
Sep 5, 2022

Conversation

hunhoffe
Copy link
Contributor

@hunhoffe hunhoffe commented Jul 12, 2022

Description

I am working on integrating the new scheduler into openwhisk-deploy-kube (see this draft PR here). While most of the changes that need to be made are within the apache/openwhisk-deploy-kube repo, some changes need to be made here for full support.

Namely:

  • Scheduler images need to be build and pushed to dockerhub
  • The scheduler and controller both need to option to use the akka cluster discovery in the case that they are replicated; the existing mechanism of using seedNodes is not a great fit for Kubernetes deployments

I tested locally (mostly using Kubernetes deployments) but I'm not sure the best way to test the tooling and changes to travis/CI.

Note that while I did, in general, try to integrate the scheduler into the tooling, I did not attempt to add it to dev/src/main/groovy/intellijRunConfig.groovy, as I don't have experience in this area.

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Scheduler
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

@hunhoffe hunhoffe force-pushed the openwhisk-deploy-kube branch from 1711ee7 to 2312798 Compare July 13, 2022 02:59
@hunhoffe hunhoffe marked this pull request as draft July 13, 2022 03:09
@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2022

Codecov Report

Attention: Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 76.42%. Comparing base (138f3d9) to head (806cbe7).
Report is 90 commits behind head on master.

Files with missing lines Patch % Lines
...rg/apache/openwhisk/core/scheduler/Scheduler.scala 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5278      +/-   ##
==========================================
- Coverage   81.03%   76.42%   -4.61%     
==========================================
  Files         239      239              
  Lines       14245    14249       +4     
  Branches      594      602       +8     
==========================================
- Hits        11543    10890     -653     
- Misses       2702     3359     +657     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hunhoffe hunhoffe marked this pull request as ready for review July 13, 2022 13:01
@hunhoffe
Copy link
Contributor Author

Apologies for making the PR live - then draft - then live again, but I believe it is in a good state now and ready for review!

@dgrove-oss
Copy link
Member

Hi Erika, have you filed an Individual Contributors License Agreement with Apache (https://www.apache.org/licenses/contributor-agreements.html#clas)? I did a quick search and I don't think I found one for you, but the tooling isn't always perfect.

If you haven't done so already please go ahead and do that.

@hunhoffe
Copy link
Contributor Author

@dgrove-oss I did submit an ICLA in the past, so I should be good to go. To be on the safe side and check for clerical errors, I have sent an email asking for confirmation that my form is on file.

@dgrove-oss
Copy link
Member

ok, cool. That's fine then. The tool for looking up ICLAs changed recently. Now you have to search by email address instead of by name and I haven't figured out how to do fuzzy searches yet.

@hunhoffe hunhoffe force-pushed the openwhisk-deploy-kube branch from 9d025b5 to 22d4da8 Compare July 21, 2022 15:43
@hunhoffe
Copy link
Contributor Author

Rebased to master so I can make sure new changes integrate well into openwhisk-deploy-kube, also - I did confirm my ICLA is good.

@hunhoffe hunhoffe force-pushed the openwhisk-deploy-kube branch from 62dee68 to c6284af Compare July 25, 2022 16:41
Copy link
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

I generally agree with the direction of this PR.
I just wanted to make sure only the required logic is added.

@hunhoffe hunhoffe force-pushed the openwhisk-deploy-kube branch from c04a9b4 to 806cbe7 Compare September 1, 2022 23:48
@hunhoffe
Copy link
Contributor Author

hunhoffe commented Sep 1, 2022

Rebased to master, finished addressing comments about changes to controller (by removing changes to the controller)

@hunhoffe hunhoffe requested a review from style95 September 2, 2022 14:22
@style95
Copy link
Member

style95 commented Sep 5, 2022

Thank you, Erika, for your effort. 👍

@style95 style95 merged commit 347ff1f into apache:master Sep 5, 2022
msciabarra pushed a commit to nuvolaris/openwhisk that referenced this pull request Nov 23, 2022
…pache#5278)

* Kubernetes Akka bootstrap for controller

* Update cluster management for the scheduler to help with k8s deployment

* Made changes to tools to try to integrate scheduler into travis/CI build
process

* Added scheduler Dockerfile.cov

* Use consistent ordering of components

* remove canonical.port setting in scheduler, controller

* Remove unneeded dependency from controller, scheduler

* Remove cluster creation from ShardingContainerPoolBalancer

* Remove trailing whitespace

Signed-off-by: Erika Hunhoff <[email protected]>

* Revert akka cluster changes to controller

Signed-off-by: Erika Hunhoff <[email protected]>

Signed-off-by: Erika Hunhoff <[email protected]>
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.

5 participants