Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Allow setting Kiam version <3.0 #1599

Merged

Conversation

davidmccormick
Copy link
Contributor

The Kiam server and agent command line flags changed in v3.0.
Although we install Kiam v3.2 by default - allow users to deploy older versions with the different command line options.

Tested deploying v2.7 and then upgrading to v3.2

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 21, 2019
@davidmccormick
Copy link
Contributor Author

@paalkr does the PR aid your cluster migration strategy?

@davidmccormick davidmccormick added this to the v0.13.0 milestone May 21, 2019
@codecov-io
Copy link

codecov-io commented May 21, 2019

Codecov Report

Merging #1599 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1599   +/-   ##
=======================================
  Coverage   25.49%   25.49%           
=======================================
  Files          97       97           
  Lines        5060     5060           
=======================================
  Hits         1290     1290           
  Misses       3625     3625           
  Partials      145      145
Impacted Files Coverage Δ
pkg/api/cluster.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a7b776...c7c2a65. Read the comment docs.

@dominicgunn
Copy link
Contributor

Slightly out of scope of this PR, but should we also update the image version of kiam in the cluster.yaml.tmpl? It's still set to a default of 2.8.

@dominicgunn
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 21, 2019
@paalkr
Copy link
Contributor

paalkr commented May 21, 2019

I think it's a good idea to let users decide which versions of kiam they would like to run, and it's definitely a good move to support backwards compatibility. When it comes to upgrade of kiam to 3.2, users have to update their kiam certs at some point. But this pr will probably make migration to 0.13 a little easier for many users, as there is just one thing less to focus on. The kiam upgrade can be carried out later, after the main cluster upgrade.

Personally I will probably make the move to kiam 3.2 during the initial cluster upgrade, as I have tested the cert update and documented in our systems how to execute the upgrade.

@k8s-ci-robot k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label May 22, 2019
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 22, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: mumoshu

If they are not already assigned, you can assign the PR to them by writing /assign @mumoshu in a comment when ready.

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

@davidmccormick
Copy link
Contributor Author

Updated cluster.yaml template as suggested and updated the default values so that it caches lookups by having a default session timeout of 30m and changing the server urls to remove the :port shenanigans.

Update Kiam template and defaults.

Change default sessontimeout to 30m by default so that Kiam does some caching.

Update tests with 30m session caching
@davidmccormick davidmccormick merged commit 00746e2 into kubernetes-retired:master May 22, 2019
@davidmccormick davidmccormick modified the milestones: v0.13.0, v0.13.0-rc.2 May 22, 2019
davidmccormick added a commit to HotelsDotCom/kube-aws that referenced this pull request May 22, 2019
Update Kiam template and defaults.

Change default sessontimeout to 30m by default so that Kiam does some caching.

Update tests with 30m session caching
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants