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

[charts/gateway]Make restman host and port modifiable for otk install job #267

Open
wants to merge 2 commits into
base: stable
Choose a base branch
from

Conversation

SebastiaanKveek
Copy link

Currently the otk-install job template does not allow to change the restman host or port. This makes it very inflexible for example in our case where we are using a custom loadbalancer and clusterhostname instead of the ingress to make restman available. It's also not possible to patch this job with --post-renderer since it's a post-upgrade,post-install job.

Small adjustment on the otk-install job template and the values file

Benefits

The benefits of this change will be to make the otk-install job more flexible by parameterizing the restman variables. It will still retain the current values as defaults so it should not be a breaking change.

Drawbacks
No drawbacks but please double check if the templating code used is correct

Applicable issues

  • fixes #

Additional information

Checklist

  • [] Chart version bumped in Chart.yaml according to semver.
  • [] Variables are documented in the README.md
  • Title of the PR starts with chart name (e.g. [charts/gateway])
  • If the chart contains a values-production.yaml apart from values.yaml, ensure that you implement the changes in both files

@APIM-KiranVaddadi
Copy link
Contributor

@SebastiaanKveek No harm in adding option to specify restman host and port but would like avoid repetition of configuration if possible. For our understanding, can share more details about the gateway service (type). OTK uses the gateway service and would like to know why its working.

@SebastiaanKveek
Copy link
Author

SebastiaanKveek commented Sep 28, 2023

@APIM-KiranVaddadi We added a loadBalancer service via the values.yml that only allow traffic on a specific port. This loadbalancer name get's templated like this:
name: {{ template "gateway.fullname" . }}
See https://github.com/CAAPIM/apim-charts/blob/stable/charts/gateway/templates/service.yaml line 11.

Another idea would be to point the otk-install job to the management load balancer service which is created by default through the helm chart. See line 12 in https://github.com/CAAPIM/apim-charts/blob/stable/charts/gateway/templates/management-service.yaml
That would mean you point towards ` name: {{ template "gateway.fullname" . }}-management'
As opposed to the current name. So then the management loadbalancer service would be default and you could still have the option of modifying it yourself.

@APIM-KiranVaddadi
Copy link
Contributor

APIM-KiranVaddadi commented Sep 28, 2023

@SebastiaanKveek Management service is optional and conditional. Its better to leave the default to be 'gateway service' if otk.restmanHost is blank and same goes for the port. We are will productize and include the changes in the next OTK or Gateway Helm release. For now we will keep this open and close when the changes goes into product. Thank you.

@APIM-KiranVaddadi APIM-KiranVaddadi added the enhancement New feature or request label Sep 28, 2023
@SebastiaanKveek
Copy link
Author

@APIM-KiranVaddadi Great thanks for the quick response and solution.

@SebastiaanKveek
Copy link
Author

Wondering what the status on this is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants