Allow changing subnet/kube annotation prefix #997
Conversation
|
@tomdee is there any way I can move it forward? |
subnet/kube/annotations.go
Outdated
| @@ -0,0 +1,60 @@ | |||
| // Copyright 2016 flannel authors | |||
subnet/kube/annotations_test.go
Outdated
| @@ -0,0 +1,96 @@ | |||
| // Copyright 2016 flannel authors | |||
subnet/kube/annotations.go
Outdated
| prefix += "-" | ||
| } | ||
|
|
||
| matches, err := regexp.MatchString(`(?:[a-z0-9_-]+\.)+[a-z0-9_-]+/(?:[a-z0-9_-]+-)?$`, prefix) |
There was a problem hiding this comment.
Can you add a brief comment explaining this regex and why it was chosen (e.g. it matches the format used by the kubernetes for annotations)
There was a problem hiding this comment.
I added a comment. LMK if you'd like to further improve it. The pattern is well tested in annotations_test.go.
| @@ -121,6 +122,7 @@ func init() { | |||
| flannelFlags.BoolVar(&opts.ipMasq, "ip-masq", false, "setup IP masquerade rule for traffic destined outside of overlay network") | |||
| flannelFlags.BoolVar(&opts.kubeSubnetMgr, "kube-subnet-mgr", false, "contact the Kubernetes API for subnet assignment instead of etcd.") | |||
| flannelFlags.StringVar(&opts.kubeApiUrl, "kube-api-url", "", "Kubernetes API server URL. Does not need to be specified if flannel is running in a pod.") | |||
| flannelFlags.StringVar(&opts.kubeAnnotationPrefix, "kube-annotation-prefix", "flannel.alpha.coreos.com", `Kubernetes annotation prefix. Can contain single slash "/", otherwise it will be appended at the end.`) | |||
There was a problem hiding this comment.
What's the reasoning behind supporting both with and without a slash?
There was a problem hiding this comment.
My idea was to add a trailing slash if the annotation doesn't contain any. Please check test cases I copied here. They show how the flag value is transformed to the final annotation name.
+ {
+ prefix: "flannel.alpha.coreos.com",
+ expectedBackendType: "flannel.alpha.coreos.com/backend-type",
+ },
+ {
+ prefix: "flannel.alpha.coreos.com/",
+ expectedBackendType: "flannel.alpha.coreos.com/backend-type",
+ },
+ {
+ prefix: "flannel.alpha.coreos.com/prefix",
+ expectedBackendType: "flannel.alpha.coreos.com/prefix-backend-type",
+ },
+ {
+ prefix: "flannel.alpha.coreos.com/prefix-",
+ expectedBackendType: "flannel.alpha.coreos.com/prefix-backend-type",
+ },
This would be useful for us. We'd like to to have prefixes like flannel.alpha.coreos.com/some-id-. I'm fine with changing that. Let me know what is your preference.
|
@tomdee PTALA |
|
@kopiczko Thanks for making the markups. The changes look good. Merging. |
Description
Fixes #804
It adds new
--kube-annotation-prefixflag which defaults toflannel.alpha.coreos.comso the current behavior remains unchanged.Todos
Release Note