Substitute federation_domain_map parameter with its value in node bootstrap scripts. #28132
Conversation
madhusudancs
added
cherrypick-candidate
release-note-action-required
…tstrap scripts. This also removes the substitution code we added to the build scripts in one of the previous commits.
madhusudancs
force-pushed
the
fed-kubedns-flags-nodebootstrap
branch
from
b00f231 to
816c4d9
Compare
k8s-github-robot
added
the
size/L
|
GCE e2e build/test passed for commit 816c4d9. |
erictune
added
the
cherry-pick-approved
matchstick
added
the
priority/critical-urgent
|
@madhusudancs was there a typo? As far as I can tell, #28040 did not break a bunch of scripts. |
|
LGTM to me - the build/common.sh change is reverted and the fixed skydns-rc.yaml templates are already committed. |
|
Looks good to me, but I haven't tested it |
|
@MikeSpreitzer yes sorry, it was a typo. I meant PR #27986 where I meant #28040. |
ghost
added
the
lgtm
|
@madhusudancs As discussed in person, please open a separate issue to get rid of the cargo-culting code style that you had to follow along with here for consistency. For this change, LGTM. |
|
GCE e2e build/test passed for commit 816c4d9. |
|
Automatic merge from submit-queue |
…gs-nodebootstrap
Automatic merge from submit-queue
Substitute federation_domain_map parameter with its value in node bootstrap scripts.
This PR also removes the substitution code we added to the build scripts.
**Release Note**
```release-note
If you use one of the kube-dns replication controller manifest in `cluster/saltbase/salt/kube-dns`, i.e. `cluster/saltbase/salt/kube-dns/{skydns-rc.yaml.base,skydns-rc.yaml.in}`, either substitute one of `__PILLAR__FEDERATIONS__DOMAIN__MAP__` or `{{ pillar['federations_domain_map'] }}` with the corresponding federation name to domain name value or remove them if you do not support cluster federation at this time. If you plan to substitute the parameter with its value, here is an example for `{{ pillar['federations_domain_map'] }`
pillar['federations_domain_map'] = "- --federations=myfederation=federation.test"
where `myfederation` is the name of the federation and `federation.test` is the domain name registered for the federation.
```
cc @erictune @kubernetes/sig-cluster-federation @MikeSpreitzer @luxas
[]()
(cherry picked from commit 3a6494e)
…gs-nodebootstrap
Automatic merge from submit-queue
Substitute federation_domain_map parameter with its value in node bootstrap scripts.
This PR also removes the substitution code we added to the build scripts.
**Release Note**
```release-note
If you use one of the kube-dns replication controller manifest in `cluster/saltbase/salt/kube-dns`, i.e. `cluster/saltbase/salt/kube-dns/{skydns-rc.yaml.base,skydns-rc.yaml.in}`, either substitute one of `__PILLAR__FEDERATIONS__DOMAIN__MAP__` or `{{ pillar['federations_domain_map'] }}` with the corresponding federation name to domain name value or remove them if you do not support cluster federation at this time. If you plan to substitute the parameter with its value, here is an example for `{{ pillar['federations_domain_map'] }`
pillar['federations_domain_map'] = "- --federations=myfederation=federation.test"
where `myfederation` is the name of the federation and `federation.test` is the domain name registered for the federation.
```
cc @erictune @kubernetes/sig-cluster-federation @MikeSpreitzer @luxas
[]()
(cherry picked from commit 3a6494e)
|
@madhusudancs @mml @nikhiljindal @quinton-hoole @girishkalele this PR has a release-note-action-required label, but I can't tell if the action is relevant anymore Specifically, CHANGELOG.md refers to kube-dns living in clusters/saltbase, and it has since moved in |
|
@spiffxp The |
|
@madhusudancs if the labels are wrong, we should correct the labels (I don't have privileges, I'm just the sanity check here) this entry shows up in the CHANGELOG.md for 1.4.0-alpha.1, and if I look at the release-1.3 branch, I still see kube-dns in its old saltbase location (https://github.com/kubernetes/kubernetes/tree/release-1.3/cluster/saltbase/salt/kube-dns) as a cluster operator looking at v1.4.0 for the first time since v1.3.0, what action would I need to take? |
|
@spiffxp This shouldn't appear in the v1.4 change logs at all. People (admins) shouldn't have to do anything special in v1.4. This was already in v1.3.0-beta.2 change log and it should remain that way. Honestly, I don't know how to make it go away from 1.4 change logs. Do you know what labels need to be applied/removed? I can apply/remove them. |
|
@madhusudancs if you don't want this to crop up in future That said, I'm still concerned this needs to be an action required entry for 1.4.0, just because the release-1.3 branch doesn't have the dns addon in the new location. Can you help me understand if I just need to call out the dns addon migration, or is there an actual change that needs to happen if I'm upgrading from v1.3.0? |
Yeah. But action was required in v1.3. Not in v1.4.
This release note is not for dns addon manifests' location change. This is a specific change within the contents of the manifest and that change was applicable in v1.3, not so relevant in v1.4. This is the PR that made the location change PR #28699 and that probably needs a There is no new "action required" as far as this change is concerned in v1.4. I would like to remove this note from 1.4. How do I remove it? |
|
@madhusudancs ok, thanks for clarifying, I'll make sure the action required is manually massaged away in the CHANGELOG.md changes I make I can't speak specifically as to why this got picked up by the relnotes script even though it's in the v1.3 milestone; if I had to guess, it probably has something to do with me misusing the tool, or the timing and method of the original If you want to work around this in the future I would just change the labels as I suggested in the previous comment, otherwise it'll take some forensics of the tools in kubernetes/release that I don't have time for right now |
Sure. Noted. |
This PR also removes the substitution code we added to the build scripts.
Release Note
cc @erictune @kubernetes/sig-cluster-federation @MikeSpreitzer @luxas