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

Allow ECR pull from controller IAM role #35

Conversation

cknowles
Copy link
Contributor

@cknowles cknowles commented Nov 5, 2016

Goes some way to resolve coreos/coreos-kubernetes/issues/620, a replacement of coreos/coreos-kubernetes#731.

@codecov-io
Copy link

codecov-io commented Nov 5, 2016

Current coverage is 56.90% (diff: 100%)

Merging #35 into master will not change coverage

@@             master        #35   diff @@
==========================================
  Files             4          4          
  Lines           949        949          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            540        540          
  Misses          329        329          
  Partials         80         80          

Powered by Codecov. Last update 45452ba...145aebb

@mumoshu
Copy link
Contributor

mumoshu commented Nov 7, 2016

@c-knowles LGTM. Thanks for your contribution!
FYI, your PRs are going to be included in v0.9.1-rc.2.

@mumoshu mumoshu merged commit 6255751 into kubernetes-retired:master Nov 7, 2016
@cknowles
Copy link
Contributor Author

cknowles commented Nov 7, 2016

Great, thanks! I just need to figure out if it's worth re-doing coreos/coreos-kubernetes#716 next. I've been using it already for a while now on a forked build.

@mumoshu
Copy link
Contributor

mumoshu commented Nov 8, 2016

@c-knowles As a kube-aws user, I'd be greatly happy if I could deploy kube-aws created clusters into existing VPCs.

After a deployment, it would be way better if services hosted in both kubernetes and existing infrastructure could start communicating each other (if we demanded so), without manual steps we have to take for now. Those manual steps seem to include (1) maintaining route tables (2) assigning it to kube-aws created subnets, etc?

After reading your great write up in coreos/coreos-kubernetes#716, I now realize that it is basically what your PR addresses, right?

In short: I'm a fan of your PR 👍

@mumoshu mumoshu mentioned this pull request Nov 8, 2016
22 tasks
@cknowles
Copy link
Contributor Author

cknowles commented Nov 8, 2016

Sure, I'll redo that one soon then. It's exactly what I'm using it for - to access RDS in the same VPC. I'm interested to get some acceptance tests in there too but can discuss in the PR once I've made it.

@pieterlange
Copy link
Contributor

/off current ECR topic

The described usecase (using existing services) is already possible by using routeTableId; the PR addressed a need for a separate routetable (and subnet) for the controller only (eg, so one could put the controller into a public subnet).

It might be better to implement this into the nodepool concept we've been talking about..

@mumoshu
Copy link
Contributor

mumoshu commented Nov 8, 2016

@pieterlange Thanks for confirmation. Really helpful 👍

So, for anyone interested, my comment #35 (comment) was missing the point.
It turns out that it was already possible doing:

  1. Add one route for each newly created subnet, to an existing route table (if specified)
  2. Associate an existing route table to newly created subnets

...so that we can make services in kubernetes and existing infrastructure communicate to each other.

And what @pieterlange described follows.

(Any correction is welcome!)

@cknowles
Copy link
Contributor Author

cknowles commented Nov 8, 2016

Let's move to #44

@cknowles cknowles deleted the allow-private-ecr-from-controller branch November 24, 2016 07:53
davidmccormick pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Jul 18, 2018
…/add-helm-deploy-operator to hcom-flavour

* commit '8162cdf3338247991eece79e8cbb0be676a885e7':
  RUN-861 Remove mantle-helm-deploy object as decided we don't want to create this from kube-aws. Correct typo
  RUN-861 Rename apiGroup based on changes I made to deploy-operator source. Rename clusterrolle + binding to match other kube controller conventions.
  Add in mantle-helm-deploy resource to (hopefully) deploy mantle immediately when the helmOperator comes online. Correct namespace of helmdeploy serviceaccount
  Make controllers create resources to run helmDeployOperator: add helmDeploy.yaml, crd.yaml & rbac.yaml to install-kube-system.service
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot pull private AWS ECR image from controller node
4 participants