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

Juju kubernetes-master: Fix UpdateAddonsTactic to use local repo #41251

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@Cynerva
Copy link
Contributor

Cynerva commented Feb 10, 2017

What this PR does / why we need it:

This PR does 2 things:

  1. When building the kubernetes-master charm, instead of cloning the Kubernetes repo from github to pull the addons, just use the local repository instead
  2. If the KUBE_VERSION environment variable is set, then clone the local repository and checkout the corresponding branch instead

We need this to guarantee that, when we are developing and building charms against a stable release, we can pull in addon versions that work with it.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged):

juju-solutions/bundle-canonical-kubernetes#213

Special notes for your reviewer:

Release note:

Juju kubernetes-master: Fix UpdateAddonsTactic to use local repo
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Feb 10, 2017

Hi @Cynerva. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

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. I understand the commands that are listed here.

@k8s-reviewable

This comment has been minimized.

Copy link

k8s-reviewable commented Feb 10, 2017

This change is Reviewable

@marcoceppi

This comment has been minimized.

Copy link
Member

marcoceppi commented Feb 10, 2017

@k8s-bot ok to test

@marcoceppi

This comment has been minimized.

Copy link
Member

marcoceppi commented Feb 10, 2017

@k8s-bot node e2e test this
@k8s-bot cri node e2e test this

I find these failures concerning, possibly flakey?

@marcoceppi

This comment has been minimized.

Copy link
Member

marcoceppi commented Feb 10, 2017

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@Cynerva

This comment has been minimized.

Copy link
Contributor

Cynerva commented Feb 10, 2017

@k8s-bot kops aws e2e test this

@lazypower

This comment has been minimized.

Copy link
Member

lazypower commented Feb 13, 2017

Fantastic, this successfully closed 213 during my testing. I'm +1

The magic incantation to run this in our CI system that builds with the appropriate templates is to have to KUBE_VERSION environment variable set, otherwise it defaults to tree, which is often some forked tip of MASTER.

/approve

@marcoceppi

This comment has been minimized.

Copy link
Member

marcoceppi commented Feb 13, 2017

/lgtm

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Feb 13, 2017

@marcoceppi: you can't LGTM a PR unless you are an assignee.

In response to this comment:

/lgtm

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. I understand the commands that are listed here.

@lazypower lazypower referenced this pull request Feb 13, 2017

Merged

Multi master patch #41351

@Cynerva

This comment has been minimized.

Copy link
Contributor

Cynerva commented Feb 21, 2017

@david-mcmahon are you able to give this a review? It would be much appreciated!

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Feb 23, 2017

[APPROVALNOTIFIER] This PR is APPROVED

The following people have approved this PR: Cynerva, chuckbutler, marcoceppi

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-merge-robot added a commit that referenced this pull request Feb 25, 2017

Merge pull request #41351 from chuckbutler/multi-master-patch
Automatic merge from submit-queue (batch tested with PRs 40665, 41094, 41351, 41721, 41843)

Multi master patch

**What this PR does / why we need it**: Corrects a sync files issue present when running in a HA Master configuration. This PR adds logic to syncronize on first deployment for `/etc/kubernetes/serviceaccount.key` which will cause cypto verification failure if not 1:1 on each master unit. Additionally syncs basic_auth and additional files in /srv/kubernetes. 

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #41019

**Special notes for your reviewer**: This requires PR #41251 as a dependency before merging. 

**Release note**:

```release-note
Juju - K8s master charm now properly keeps distributed master files in sync for an HA control plane.
```
@Cynerva

This comment has been minimized.

Copy link
Contributor

Cynerva commented Feb 25, 2017

Closing since this change was merged in via #41351

@Cynerva Cynerva closed this Feb 25, 2017

@Cynerva Cynerva deleted the Cynerva:gkk/fix-update-addons-tactic branch Feb 25, 2017

k8s-merge-robot added a commit that referenced this pull request Mar 4, 2017

Merge pull request #41919 from Cynerva/gkk/kubelet-auth
Automatic merge from submit-queue (batch tested with PRs 41919, 41149, 42350, 42351, 42285)

Juju: Disable anonymous auth on kubelet

**What this PR does / why we need it**:

This disables anonymous authentication on kubelet when deployed via Juju.

I've also adjusted a few other TLS options for kubelet and kube-apiserver. The end result is that:
1. kube-apiserver can now authenticate with kubelet
2. kube-apiserver now verifies the integrity of kubelet

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*:

juju-solutions/bundle-canonical-kubernetes#219

**Special notes for your reviewer**:

This is dependent on PR #41251, where the tactics changes are being merged in separately.

Some useful pages from the documentation:
* [apiserver -> kubelet](https://kubernetes.io/docs/admin/master-node-communication/#apiserver---kubelet)
* [Kubelet authentication/authorization](https://kubernetes.io/docs/admin/kubelet-authentication-authorization/)

**Release note**:

```release-note
Juju: Disable anonymous auth on kubelet
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment