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

Controller node should be tagged appropriately #370

Closed
redbaron opened this issue Mar 1, 2017 · 9 comments
Closed

Controller node should be tagged appropriately #370

redbaron opened this issue Mar 1, 2017 · 9 comments
Labels
open for pull request
Milestone
v0.9.5-rc.5

Comments

@redbaron
Copy link
Contributor

redbaron commented Mar 1, 2017

Apparently there are some well-known-but-not-documented-tags which are taken into consideration by AWS cloud provide, one example of it would be:
https://github.com/kubernetes/kubernetes/pull/41702/files#diff-07ba008af9c76b0539556ff7fde3105eR162

so we should follow the pattern and tag our nodes
@mumoshu
Copy link
Contributor

mumoshu commented Mar 1, 2017

Thanks as always @redbaron 👍
After looking into the code, "kubernetes.io/role/master" and "k8s.io/role/master" referred in the PR you've shared are only tags seems to be used as of today.
So just let's tag our with those.

@mumoshu mumoshu added this to the v0.9.5-rc.5 milestone Mar 20, 2017
mumoshu added a commit to mumoshu/kube-aws that referenced this issue Mar 21, 2017
@mumoshu
Tag controller nodes appropriately
d22a7a5 
with `kubernetes.io/role`.
Resolves kubernetes-retired#370
@mumoshu mumoshu mentioned this issue Mar 21, 2017
Merged
camilb added a commit to camilb/kube-aws that referenced this issue Apr 5, 2017
@camilb
Merge remote-tracking branch 'kubernetes-incubator/master'
7f93c09 
* kubernetes-incubator/master: (29 commits)
  Emit errors when kube-aws sees unexpected keys in cluster.yaml Resolves kubernetes-retired#404
  Tag controller nodes appropriately with `kubernetes.io/role`. Resolves kubernetes-retired#370
  Make Container Linux AMI fetching a bit more reliable
  Stop locksmithd errors on etcd nodes
  Upgrade heapster to version 1.3.0 (kubernetes-retired#420)
  Auth token file support (kubernetes-retired#418)
  Update README.md
  Update README accordingly to the new git repo
  AWS China region support (kubernetes-retired#390)
  Conform as a Kubernetes Incubator Project
  Fixed typo in template
  upgrade aws-sdk to latest version Fix kubernetes-retired#388
  Upgrade Kubernetes version to v1.5.4
  Fix assumed public hostnames for EC2 instances in us-east-1
  Fix assumed public hostnames for EC2 instances in us-east-1
  typo
  fix: etcdDataVolumeEncrypted not creating encrypted volumes fixes kubernetes-retired#383
  Allow disabling wait signals fixes kubernetes-retired#371
  Update file paths in readme
  Fix an issue with glue security group documentation
  ...
@zhangtiny123
Copy link

Hi @redbaron @mumoshu
I'm facing an error when I fetch the instance data and store it into mongodb. The dotted field "kubernetes.io/role/master" is not valid for storage

For me this tag does not make any sense, but an error ...
Since the value of this tag is empty, what do you think about tagging with role: kubernetes.io/role/master or something like that?

@redbaron
Copy link
Contributor Author

redbaron commented May 4, 2017

@zhangtiny123 controllers are tagged with tag expected by AWS cloud provider code in Kubernetes, so we can't change it

@redbaron
Copy link
Contributor Author

redbaron commented May 4, 2017

look at the link in the first message

@mumoshu
Copy link
Contributor

mumoshu commented May 8, 2017

@zhangtiny123 As @redbaron kindly described, we don't seem to be able to fix/avoid it from k8s side.

AFAIK, you'd need to flatten retrieved object before updating MongoDB with them like mentioned in a SO question here so that you can still query your MongoDB by something like db.find({'node.spec.labels.kubernets.io/role/master': ''})?

@zhangtiny123
Copy link

@mumoshu Thanks for your suggestion, but it seems not fit my situation...

Currently I just delete the tag kubernetes.io/role/master from my cluster manually, since it seems there's no one else reference the tag, and everything looks OK , but is there any hidden danger?

@redbaron
Copy link
Contributor Author

redbaron commented May 9, 2017

@zhangtiny123 , just a heads up, that there is new tagging scheme in Kube 1.6, where ALL resources are tagged with kubernetes.io/cluster/<CLUSTER_NAME> , and you better not to remove any of those tags, otherwise at least LoadBalancer services wont be able to create ELBs

@mumoshu
Copy link
Contributor

mumoshu commented May 9, 2017

@zhangtiny123 Thanks for the reply.
Just a quick thought but can you script to replace every dot with a hyphen(-) or maybe underscore(_) after you've fetched the instance data? so that dots on original keys won't affect MongoDB

@zhangtiny123
Copy link

@mumoshu Thanks man, good idea !

@redbaron I've already add it back... thanks for your patience

kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this issue Mar 27, 2018
@mumoshu
Tag controller nodes appropriately
bcafe3e 
with `kubernetes.io/role`.
Resolves kubernetes-retired#370
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
open for pull request
Projects
None yet
Milestone
v0.9.5-rc.5
Development

No branches or pull requests
3 participants
@redbaron @mumoshu @zhangtiny123