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.

Sign up for GitHub

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

Jump to bottom

Merge v1.3 branch into master [2017-05-11] #1548

Merged
merged 3 commits into from
May 11, 2017
Merged

Merge v1.3 branch into master [2017-05-11] #1548

merged 3 commits into from
May 11, 2017

Conversation

timoreimann
Copy link
Contributor

@timoreimann timoreimann commented May 4, 2017 •
edited

Brings #1581 into mainline.

@timoreimann timoreimann force-pushed the kubernetes-ignore-missing-pass-host-header-annotation branch from 61ce4dd to 8791de1 Compare May 4, 2017 09:40
@timoreimann timoreimann force-pushed the kubernetes-ignore-missing-pass-host-header-annotation branch from 8791de1 to 3c0a26b Compare May 5, 2017 17:26
dtomcej
dtomcej reviewed May 9, 2017
View reviewed changes
Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM

:shipit:

@timoreimann
[Kubernetes] Ignore missing pass host header annotation.
fe6c35b 
A missing annotation would previously be handled in the default error
case, causing a noisy warning-level log message to be generated each
time.

We add another case statement to ignore the case where the annotation is
missing from the annotations map.

Also piggybacking a minor improvement to the log message.
@timoreimann timoreimann mentioned this pull request May 10, 2017
Merged
@timoreimann timoreimann added this to the 1.4 milestone May 10, 2017
@timoreimann timoreimann added kind/bug/confirmed a confirmed bug (reproducible). and removed kind/enhancement a new or improved feature. labels May 10, 2017
@timoreimann
Copy link
Contributor Author

I believe we should have this be part of 1.3 as well, so I cherry-picked the fix.

See PR 1581.

@timoreimann timoreimann force-pushed the kubernetes-ignore-missing-pass-host-header-annotation branch from 3c0a26b to 3307d27 Compare May 10, 2017 17:40
@timoreimann
Merge pull request traefik#1581 from timoreimann/1-3-kubernetes-ignor…
d2b204a 
…e-missing-pass-host-header-annotation

[Kubernetes] Ignore missing pass host header annotation. [v1.3 - CHERRY-PICK]
@timoreimann timoreimann force-pushed the kubernetes-ignore-missing-pass-host-header-annotation branch from 3307d27 to b40492b Compare May 11, 2017 16:11
vdemeester
vdemeester approved these changes May 11, 2017
View reviewed changes
Copy link
Contributor

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐮

ldez
ldez requested changes May 11, 2017
View reviewed changes
Copy link
Member

@ldez ldez left a comment •
edited

Choose a reason for hiding this comment

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

It's a good idea to add this modification in 1.3 branch and the master

@timoreimann
Copy link
Contributor Author

@ldez:

It's a good idea to add this modification in 1.3 branch and the master

This PR handles master, and #1581 has been merged into the 1.3 branch earlier today. Anything missing from your point?

@ldez ldez removed this from the 1.4 milestone May 11, 2017
@ldez
Copy link
Member

ldez commented May 11, 2017

@timoreimann sorry mispelling

It's not a good idea to add this modification in 1.3 branch and the master

@emilevauge
Copy link
Member

@timoreimann We still use the same process:

  1. Bug fix PR on the release branch (here v1.3)
  2. Merge v1.3 into master later

As #1581 has already been merged into v1.3, I propose to simply close this one. OK for you @timoreimann ?

@timoreimann timoreimann force-pushed the kubernetes-ignore-missing-pass-host-header-annotation branch from b40492b to 3112432 Compare May 11, 2017 19:13
@timoreimann timoreimann changed the title [Kubernetes] Ignore missing pass host header annotation. Merge v1.3 branch into master [2017-05-11] May 11, 2017
@timoreimann timoreimann removed area/provider/k8s/ingress kind/bug/confirmed a confirmed bug (reproducible). labels May 11, 2017
@timoreimann
Copy link
Contributor Author

@emilevauge @ldez I repurposed this PR for the merge. Please have a look again.

@ldez
Copy link
Member

ldez commented May 11, 2017

:trollface: In the @timoreimann head: if I change the PR title, they don't see nothing...
timoreimann:kubernetes-ignore-missing-pass-host-header-annotation is not the v1.3 branch, or I have miss somethings?

@timoreimann
Copy link
Contributor Author

@ldez Locally I merged upstream/v1.3 into upstream/master and pushed the result to timoreimann:kubernetes-ignore-missing-pass-host-header-annotation. It's a shame Github won't let me change PR branches, thereby uncovering my rebranding operation. :-)

The commits look okay to me though. Or did I miss something?

emilevauge
emilevauge approved these changes May 11, 2017
View reviewed changes
Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

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

Thanks @timoreimann :)
LGTM

@timoreimann
Copy link
Contributor Author

@ldez okay for you?

ldez
ldez approved these changes May 11, 2017
View reviewed changes
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM 🚥

@ldez ldez merged commit b24b5e2 into traefik:master May 11, 2017
@ldez
Copy link
Member

ldez commented May 11, 2017

oops so quick...

@timoreimann
Copy link
Contributor Author

Thanks 👏

@timoreimann timoreimann deleted the kubernetes-ignore-missing-pass-host-header-annotation branch May 11, 2017 22:59
@timoreimann timoreimann removed the request for review from errm May 11, 2017 23:00
@ldez ldez added this to the 1.4 milestone May 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtomcej dtomcej dtomcej left review comments

@vdemeester vdemeester vdemeester approved these changes

@ldez ldez ldez approved these changes

@emilevauge emilevauge emilevauge approved these changes

Assignees
No one assigned
Labels
size/S
Projects
None yet
Milestone
1.4
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@timoreimann @ldez @emilevauge @vdemeester @dtomcej