Expose livenessProbe and readinessProbe headers to consumer (in stable/wordpress) by JaredReisinger · Pull Request #9390 · helm/charts

JaredReisinger · 2018-11-20T01:01:44Z

What this PR does / why we need it:

Problem: Despite docs that say otherwise, Kubernetes treats a 302 HTTP response as "failed" in liveness and readiness probes. (See kubernetes/kubernetes#47893.) When using ingress-terminated HTTPS, it is common for HTTP requests to WordPress to result in 302-redirects to the HTTPS equivalent. As a result, the container is considered unhealthy/unready.

Solution: Expose the httpHeaders property of the httpGet in the probe definition so that consumers can, for example, pass the X-Forwarded-Proto: https header to prevent the 302 and instead get a 200 response status. The headers are exposed as optional livenessProbeHeaders and readinessProbeHeaders arrays, and the httpHeaders array is only included in the deployment probe definitions when the respective probe headers are defined.

Signed-off-by: Jared Reisinger jaredreisinger@hotmail.com

Which issue this PR fixes

(no existing issues)

Special notes for your reviewer:

Update deployment.yaml to include optional headers.
Update values.yaml and values-production.yaml with commented-out examples.
Update README.md with configuration options, and a more detailed explanation in the Ingress section.
Update Chart.yaml version number from 3.3.0 to 3.3.1.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

DCO signed
Chart Version bumped
Variables are documented in the README.md

**Problem:** Despite docs that say otherwise, Kubernetes treats a 302 HTTP response as "failed" in liveness and readiness probes. When using ingress-terminated HTTPS, it is common for HTTP requests to WordPress to result in 302-redirects to the HTTPS equivalent. As a result, the container is considered unhealthy/unready. **Solution:** Expose the `httpHeaders` property of the `httpGet` in the probe definition so that consumers can, for example, pass the `X-Forwarded-Proto: https` header to prevent the 302 and instead get a 200 response status. The headers are exposed as optional `livenessProbeHeaders` and `readinessProbeHeaders` arrays, and only included in the deployment definition when defined. * Update `deployment.yaml` to include optional headers. * Update `values.yaml` and `values-production.yaml` with commented-out examples. * Update `README.md` with configuration options, and a more detailed explanation in the Ingress section. * Update `Chart.yaml` version number from `3.3.0` to `3.3.1`. Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>

tompizmor · 2018-11-29T12:31:11Z

LGTM @JaredReisinger,

Can you fix the conflicts in the stable/wordpress/Chart.yaml file so this can be merged?

Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>

JaredReisinger · 2018-12-22T05:49:09Z

Sorry for all the noise... I managed to push two commits without the --signoff, and it took a bit for me to unroll the madness. It's all good now!

tompizmor · 2019-01-02T07:58:20Z

/ok-to-test
/lgtm

tompizmor · 2019-01-02T11:42:55Z

/retest

JaredReisinger · 2019-01-04T02:06:36Z

/retest

JaredReisinger · 2019-01-04T02:09:44Z

Well... that didn't help. The first run failed due to a "volume multi attach" problem, and the retest may have run into the chart version having already been updated once. :-( Any suggestion?

tompizmor · 2019-01-04T07:22:03Z

@JaredReisinger you need to bump the chart version again. In this PR you changed it to 5.0.3 but that version is already committed in master because of a different PR has been merged.
You need to change it to 5.0.4

stable/wordpress/Chart.yaml

Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>

tompizmor

/ok-to-test
/lgtm

k8s-ci-robot · 2019-01-07T07:45:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JaredReisinger, tompizmor

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~stable/wordpress/OWNERS~~ [tompizmor]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

JaredReisinger · 2019-01-09T01:03:09Z

Thanks, @tompizmor!

…e/wordpress) (helm#9390) * Expose livenessProbe and readinessProbe headers to consumer **Problem:** Despite docs that say otherwise, Kubernetes treats a 302 HTTP response as "failed" in liveness and readiness probes. When using ingress-terminated HTTPS, it is common for HTTP requests to WordPress to result in 302-redirects to the HTTPS equivalent. As a result, the container is considered unhealthy/unready. **Solution:** Expose the `httpHeaders` property of the `httpGet` in the probe definition so that consumers can, for example, pass the `X-Forwarded-Proto: https` header to prevent the 302 and instead get a 200 response status. The headers are exposed as optional `livenessProbeHeaders` and `readinessProbeHeaders` arrays, and only included in the deployment definition when defined. * Update `deployment.yaml` to include optional headers. * Update `values.yaml` and `values-production.yaml` with commented-out examples. * Update `README.md` with configuration options, and a more detailed explanation in the Ingress section. * Update `Chart.yaml` version number from `3.3.0` to `3.3.1`. Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com> * bump version Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com> * bump version Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>

helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 20, 2018

k8s-ci-robot requested review from prydonius and sameersbn November 20, 2018 01:01

JaredReisinger changed the title ~~Expose livenessProbe and readinessProbe headers to consumer~~ Expose livenessProbe and readinessProbe headers to consumer (in stable/wordpress) Nov 20, 2018

helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). labels Dec 22, 2018

JaredReisinger force-pushed the tweak-probes branch from 9c86b34 to b032d02 Compare December 22, 2018 05:44

helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 22, 2018

JaredReisinger added 2 commits December 21, 2018 21:47

Merge branch 'master' into tweak-probes

91964a1

Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>

bump version

d472343

Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>

JaredReisinger force-pushed the tweak-probes branch from b032d02 to d472343 Compare December 22, 2018 05:48

helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 22, 2018

k8s-ci-robot added ok-to-test lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 2, 2019

tompizmor reviewed Jan 4, 2019

View reviewed changes

stable/wordpress/Chart.yaml Outdated Show resolved Hide resolved

bump version

e5a9dd0

Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>

k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 5, 2019

helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 5, 2019

Merge branch 'master' into tweak-probes

31fc0e2

Signed-off-by: Jared Reisinger <jaredreisinger@hotmail.com>

helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 5, 2019

tompizmor approved these changes Jan 7, 2019

View reviewed changes

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 7, 2019

k8s-ci-robot merged commit d2cd36b into helm:master Jan 7, 2019

juan131 mentioned this pull request Apr 22, 2019

[stable/wordpress] Readiness and liveness probe failes with multisite #13176

Closed

JaredReisinger deleted the tweak-probes branch July 16, 2019 22:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose livenessProbe and readinessProbe headers to consumer (in stable/wordpress)#9390

Expose livenessProbe and readinessProbe headers to consumer (in stable/wordpress)#9390
k8s-ci-robot merged 5 commits intohelm:masterfrom
JaredReisinger:tweak-probes

JaredReisinger commented Nov 20, 2018 •

edited

Loading

Uh oh!

tompizmor commented Nov 29, 2018

Uh oh!

JaredReisinger commented Dec 22, 2018

Uh oh!

tompizmor commented Jan 2, 2019

Uh oh!

tompizmor commented Jan 2, 2019

Uh oh!

JaredReisinger commented Jan 4, 2019

Uh oh!

JaredReisinger commented Jan 4, 2019

Uh oh!

tompizmor commented Jan 4, 2019

Uh oh!

Uh oh!

tompizmor left a comment

Uh oh!

k8s-ci-robot commented Jan 7, 2019

Uh oh!

JaredReisinger commented Jan 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

JaredReisinger commented Nov 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue this PR fixes

Special notes for your reviewer:

Checklist

Uh oh!

tompizmor commented Nov 29, 2018

Uh oh!

JaredReisinger commented Dec 22, 2018

Uh oh!

tompizmor commented Jan 2, 2019

Uh oh!

tompizmor commented Jan 2, 2019

Uh oh!

JaredReisinger commented Jan 4, 2019

Uh oh!

JaredReisinger commented Jan 4, 2019

Uh oh!

tompizmor commented Jan 4, 2019

Uh oh!

Uh oh!

tompizmor left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 7, 2019

Uh oh!

JaredReisinger commented Jan 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JaredReisinger commented Nov 20, 2018 •

edited

Loading