-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Autogenerate route names if host is not specified - second version with plugin based allocator support #1291
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ok to pass the regular pointer here
|
@smarterclayton - made the changes as per the review comments. See commit: 7e596f5 Updated integration test info: Thx. |
|
I just realized - if you make the default suffx "router.default.local" we can add something to the service the router command generates and auto resolve DNS to it. Oh wow. Do that, let me look at the DNS gen and see if I can get wildcard to resolve quickly.
|
|
nice! |
|
Paul, any final comments here? Does this fit with the long term sharding strat, or do you see anything fundamental we're missing? |
|
LGTM 👍 For actual sharded routers we can make a plugin that drives its config by being aware of routers and what shard keys they support. |
|
Looks to be some UI failure or timeout. Retesting. |
|
[test] |
1 similar comment
|
[test] |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1295/) |
|
LGTM [merge] |
based on service and namespace and hook it into the route processing [GOFM].
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1168/) (Image: devenv-fedora_1034) |
|
@smarterclayton / @pweil- had to rebase due to git merge conflicts , can you please redo the merge - I don't have permissions. Thx. |
|
[merge] |
|
Evaluated for origin up to d0d6feb |
|
Origin Action Required: Please contact #openshift-dev to have this pull request manually reviewed and tested |
Merged by openshift-bot
|
Changes Unknown when pulling d0d6feb on ramr:hostgen into * on openshift:master*. |
Autogenerate route names (if none is specified) using allocator plugins (currently defaults to using the simple single/unsharded router allocation scheme).
@smarterclayton / @pweil- PTAL - This is version 2 of the original PR : #1250
Unit tests pass (make check) and integration tested this with the sample app https://github.com/ramr/origin/tree/master/examples/sample-app - removing the "host" defined for the frontend in examples/sample-app/application-template-stibuild.json and getting one to autogenerate based on the service + namespace.