Skip to content
This repository has been archived by the owner on Nov 15, 2022. It is now read-only.

add support for OpenShift Routes #366

Merged
merged 1 commit into from
Oct 23, 2017
Merged

Conversation

concaf
Copy link
Collaborator

@concaf concaf commented Oct 17, 2017

This commit adds support for defining OpenShift routes in a Kedge
file.

Routes can be defined like the following at the root level of a
Kedge file

routes:
- name: route1
  host: route1-myproject.192.168.42.69.nip.io
  port:
    targetPort: 8080
  to:
    kind: Service
    name: httpd

This is implemented by essentially merging RouteSpec and ObjectMeta
and adding an array of the resulting struct at the root level under
the field "routes"

Adds docs, tests.

Fixes #238

@concaf
Copy link
Collaborator Author

concaf commented Oct 17, 2017

  • add docs
  • add tests
  • add examples

@@ -65,12 +66,20 @@ func GetScheme() (*runtime.Scheme, error) {
// Initializing the scheme with the core v1 api
scheme := api.Scheme

// TODO: find a way where we don't have to add all the subsequent schemes
Copy link
Member

Choose a reason for hiding this comment

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

Why different schema for different controllers?
Using one with everything should be fine or not?

Copy link
Collaborator Author

@concaf concaf Oct 17, 2017

Choose a reason for hiding this comment

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

It's fine, but it'd lead to a huge schema. So this is more of an optimization TODO, does not really matter much right now tbh.

@concaf
Copy link
Collaborator Author

concaf commented Oct 18, 2017

ping @kadel @cdrage @surajssd - I've added docs, tests and an example.
PTAL!
I might not be online tomorrow and day after (19th, 20th October), so feel free to edit the PR (edits are allowed by maintainers)
I've also broken changes into different commits, so should be easier to review.
If you folks need to merge, use the "Squash and merge" button, just in case 🎉

@@ -0,0 +1,13 @@
name: httpd
containers:
- image: centos/httpd
Copy link
Member

Choose a reason for hiding this comment

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

This image uses root,so it can't be run on default openShift configuration :(

Copy link
Member

@kadel kadel Oct 18, 2017

Choose a reason for hiding this comment

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

but you could use bitnami/nginx for example

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

to:
kind: Service
name: httpd
weight: 100
Copy link
Member

Choose a reason for hiding this comment

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

I would remove weight and wildcardPolicy It just makes it look too complicated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

Example:
```yaml
name: webroute
host: httpd-web.192.168.42.69.nip.io
Copy link
Member

Choose a reason for hiding this comment

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

can we maybe remove host?
It is not required, and if someone tries it like this it won't work

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agree, removed

@@ -225,6 +252,19 @@ func (app *ControllerFields) createIngresses() ([]runtime.Object, error) {
return ings, nil
}

func (app *ControllerFields) createRoutes() ([]runtime.Object, error) {
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to have tests for this function ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

Copy link
Member

@kadel kadel left a comment

Choose a reason for hiding this comment

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

Just a few notes regarding docs, and test. The code itself looks good.

This commit adds support for defining OpenShift routes in a Kedge
file.

Routes can be defined like the following at the root level of a
Kedge file

routes:
- name: route1
  host: route1-myproject.192.168.42.69.nip.io
  port:
    targetPort: 8080
  to:
    kind: Service
    name: httpd

This is implemented by essentially merging RouteSpec and ObjectMeta
and adding an array of the resulting struct at the root level under
the field "routes"

Adds docs, tests.

Fixes kedgeproject#238
Copy link
Collaborator Author

@concaf concaf left a comment

Choose a reason for hiding this comment

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

@kadel made the changes, PTAL!

@concaf concaf merged commit 95260f8 into kedgeproject:master Oct 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants