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

[bitnami/contour] Add ability to specify ingressClass and restart pods on config change #3367

Merged
merged 4 commits into from
Aug 10, 2020

Conversation

mkilchhofer
Copy link
Contributor

@mkilchhofer mkilchhofer commented Aug 8, 2020

Description of the change
As mentioned in the documentation on projectcontour.io, Contour supports watching a specific ingressClass only.

Benefits

  • Also as mentioned in the docs above:

    This can be useful while you are migrating from another controller, or if you need multiple instances of Contour.

  • Contour pods are automatically restarted after we need to change configInline
  • While I was already editing this chart I updated some missing settings in README.md

Possible drawbacks
-

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. [bitnami/chart])
  • If the chart contains a values-production.yaml apart from values.yaml, ensure that you implement the changes in both files

⚠️ Keep in mind that if you want to make changes to the kubeapps chart, please implement them in the kubeapps repository. This is only a synchronized mirror.

Signed-off-by: Marco Kilchhofer <marco@kilchhofer.info>
Signed-off-by: Marco Kilchhofer <marco@kilchhofer.info>
@mkilchhofer mkilchhofer force-pushed the feature/contour_ingressClass branch from 7ab9c5b to af48e46 Compare August 9, 2020 10:21
@mkilchhofer mkilchhofer changed the title [bitnami/contour] Add ability to specify ingressClass [bitnami/contour] Add ability to specify ingressClass and restart pods on config change Aug 9, 2020
@mkilchhofer
Copy link
Contributor Author

mkilchhofer commented Aug 9, 2020

As far as I can see the tests are failling because envoy DaemonSet has 300sec to terminate (Line 305 in values.yaml):

terminationGracePeriodSeconds: 300
Deleting release 'contour-gmywl0vz3q'...
>>> helm uninstall contour-gmywl0vz3q --namespace contour-gmywl0vz3q
release "contour-gmywl0vz3q" uninstalled
Deleting namespace 'contour-gmywl0vz3q'...
>>> kubectl delete namespace contour-gmywl0vz3q --timeout 180s
namespace "contour-gmywl0vz3q" deleted
error: timed out waiting for the condition on namespaces/contour-gmywl0vz3q
Namespace 'contour-gmywl0vz3q' did not terminate after 180s.
>>> kubectl get namespace contour-gmywl0vz3q
Namespace 'contour-gmywl0vz3q' did not terminate after 180s.
Force-deleting everything...
>>> kubectl delete all --namespace contour-gmywl0vz3q --all --force --grace-period=0
pod "contour-gmywl0vz3q-envoy-tpd2l" force deleted
warning: Immediate deletion does not wait for confirmation that the running resource has been terminated. The resource may continue to run on the cluster indefinitely.
>>> kubectl get namespace contour-gmywl0vz3q
>>> kubectl get namespace contour-gmywl0vz3q --output=json
Running 'kubectl proxy' on port 37631
>>> kubectl proxy --port=37631
Removing finalizers from namespace 'contour-gmywl0vz3q'...
>>> kubectl get namespace contour-gmywl0vz3q
Error: Error installing charts: Error processing charts
Namespace 'contour-gmywl0vz3q' terminated.

As far as I can see, we have these options to fix the failling tests:

Signed-off-by: Marco Kilchhofer <marco@kilchhofer.info>
@mkilchhofer mkilchhofer force-pushed the feature/contour_ingressClass branch from 73499c7 to f1ceae8 Compare August 9, 2020 20:15
Signed-off-by: Marco Kilchhofer <marco@kilchhofer.info>
@marcosbc
Copy link
Contributor

Hi @mkilchhofer, thanks for the contribution! The change looks fine, I'm proceeding to merge it.

As for the tests, we recently enabled PR chart testing so some failures like these are expected until we polish things up. Note that we run more reliable tests in our internal pipeline, and in the case of Contour they're currently succeeding.

@marcosbc marcosbc merged commit 5f45523 into bitnami:master Aug 10, 2020
@mkilchhofer
Copy link
Contributor Author

Hi @marcosbc , you are welcome. Thanks for accepting this change 😎

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.

2 participants