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

Track upstream v3-release tag, use loopback IP for probes #10324

Merged
merged 3 commits into from
Feb 11, 2019
Merged

Track upstream v3-release tag, use loopback IP for probes #10324

merged 3 commits into from
Feb 11, 2019

Conversation

hakamadare
Copy link
Contributor

What this PR does / why we need it:

@ewbankkit

This addresses two issues that I encountered when using this chart to
deploy kiam v3 on EKS:

  1. kiam v3.0 was released 2018-12-06 per
    https://github.com/uswitch/kiam/blob/master/CHANGELOG.md, it probably
    doesn't make sense to track a RC tag
  2. Liveness/readiness probes need to use 127.0.0.1 rather than
    localhost (see fix(kiam-server-manifests): change localhost ref uswitch/kiam#163) because of a TLS change

Signed-off-by: Steve Huff [email protected]

Which issue this PR fixes

Special notes for your reviewer:

Checklist

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

  • DCO signed
  • Chart Version bumped

@helm-bot helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 1, 2019
@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 1, 2019
@mbarrien
Copy link
Contributor

mbarrien commented Jan 1, 2019

README.md needs update too; still shows default image tags as v2.8

@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 2, 2019
@hakamadare
Copy link
Contributor Author

@mbarrien good catch, thanks! fixed now

@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 3, 2019
@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 4, 2019
@hakamadare
Copy link
Contributor Author

/assign @scottrigby

@hakamadare
Copy link
Contributor Author

/assign mgoodness

ewbankkit
ewbankkit reviewed Jan 8, 2019
View reviewed changes
stable/kiam/Chart.yaml Outdated Show resolved Hide resolved
@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 10, 2019
@hakamadare
Copy link
Contributor Author

/assign ewbankkit

@k8s-ci-robot
Copy link
Contributor

@hakamadare: GitHub didn't allow me to assign the following users: ewbankkit.

Note that only helm members and repo collaborators can be assigned and that issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign ewbankkit

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ewbankkit
Copy link
Contributor

@hakamadare Please resolve the conflict on Chart.yaml.

@cheeseandcereal
Copy link

cheeseandcereal commented Jan 15, 2019 •
edited
Loading

Please note: uswitch/kiam#162 (comment)

This upgrade can (and probably will) break compatibility with many existing TLS certificates that were previously required to have localhost as alternative domain names.

@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 16, 2019
@cpanato
Copy link
Member

cpanato commented Jan 28, 2019

/ok-to-test

@cpanato
Copy link
Member

cpanato commented Jan 28, 2019

/test pull-charts-e2e

@helm-bot helm-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 29, 2019
@helm-bot helm-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 29, 2019
@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 29, 2019
@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 30, 2019
@hakamadare
Copy link
Contributor Author

@cpanato thanks for taking a look at this kiam chart PR! i've addressed the issue that was causing the e2e tests to fail.

@cpanato
Copy link
Member

cpanato commented Feb 9, 2019

@hakamadare please update your branch and resolve the conflict thanks!

@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 11, 2019
@hakamadare
Copy link
Contributor Author

@cpanato fixed, thanks!

is there any reason why you didn't feel comfortable resolving the conflict? it's exactly the same obvious change as i resolved here (#10324) and also here (#10324). this PR allows edits from maintainers. i suspect that if any other change is merged that touches this chart, this conflict will be reintroduced, and we will be right back in this situation again.

i'm sorry, i'm travelling right now, and i cannot promise rapid response.

thanks!

@cpanato
Copy link
Member

cpanato commented Feb 11, 2019

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, hakamadare

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added 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 Feb 11, 2019
@k8s-ci-robot k8s-ci-robot merged commit 1030924 into helm:master Feb 11, 2019
tbuchier pushed a commit to tbuchier/charts that referenced this pull request Feb 14, 2019
@hakamadare @tbuchier
Track upstream v3-release tag, use loopback IP for probes (helm#10324)
1c5997b 
* Track uswitch/kiam:v3.0, use loopback for probes

Signed-off-by: Steve Huff <[email protected]>

* Update test certs for e2e per upstream doc

Signed-off-by: Steve Huff <[email protected]>
@hakamadare hakamadare deleted the kiam_v3_localhost branch February 25, 2019 14:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ewbankkit ewbankkit ewbankkit left review comments

@mgoodness mgoodness Awaiting requested review from mgoodness

@scottrigby scottrigby Awaiting requested review from scottrigby

Assignees

@scottrigby scottrigby

@prydonius prydonius

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. ok-to-test size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[stable/kiam] Add support for final kiam v3.0 release
10 participants
@hakamadare @mbarrien @k8s-ci-robot @ewbankkit @cheeseandcereal @cpanato @scottrigby @prydonius @mgoodness @helm-bot