Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Default FEDERATION_KUBE_CONTEXT to FEDERATION_NAME in federation e2e up/down scripts. #44117

Merged
merged 1 commit into from
Apr 7, 2017
Merged

Default FEDERATION_KUBE_CONTEXT to FEDERATION_NAME in federation e2e up/down scripts. #44117

merged 1 commit into from
Apr 7, 2017

Conversation

madhusudancs
Copy link
Contributor

This is consistent with how kubefed creates kubeconfig contexts.

Release note:
-->

NONE

@madhusudancs madhusudancs added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. sig/federation labels Apr 5, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 5, 2017
@madhusudancs madhusudancs added this to the v1.7 milestone Apr 5, 2017
@k8s-github-robot k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Apr 5, 2017
@madhusudancs
Copy link
Contributor Author

@k8s-bot pull-kubernetes-federation-e2e-gce test this

@k8s-reviewable
Copy link

This change is Reviewable

@madhusudancs madhusudancs force-pushed the fed-e2e-name-for-parallel-presubmit branch from d357d66 to 08beb82 Compare April 5, 2017 23:43
@madhusudancs
Copy link
Contributor Author

@k8s-bot pull-kubernetes-federation-e2e-gce test this

@k8s-github-robot k8s-github-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 5, 2017
@madhusudancs
Default FEDERATION_KUBE_CONTEXT to FEDERATION_NAME in federation e2e …
5af5378 
…up/down scripts.

This is consistent with how kubefed creates kubeconfig contexts.
@madhusudancs madhusudancs force-pushed the fed-e2e-name-for-parallel-presubmit branch from 08beb82 to 5af5378 Compare April 6, 2017 01:47
@madhusudancs
Copy link
Contributor Author

@k8s-bot pull-kubernetes-federation-e2e-gce test this

@madhusudancs
Copy link
Contributor Author

@k8s-bot non-cri e2e test this

@madhusudancs
Copy link
Contributor Author

@k8s-bot pull-kubernetes-federation-e2e-gce test this

@madhusudancs
Copy link
Contributor Author

/assign @shashidharatd

@shashidharatd
Copy link

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 7, 2017
@madhusudancs
Copy link
Contributor Author

/assign @fejta

fejta
fejta reviewed Apr 7, 2017
View reviewed changes
federation/cluster/common.sh
@@ -24,7 +24,7 @@ source "${KUBE_ROOT}/cluster/kube-util.sh"
# kubefed configuration
FEDERATION_NAME="${FEDERATION_NAME:-e2e-federation}"
FEDERATION_NAMESPACE=${FEDERATION_NAMESPACE:-federation-system}
FEDERATION_KUBE_CONTEXT="${FEDERATION_KUBE_CONTEXT:-e2e-federation}"
FEDERATION_KUBE_CONTEXT="${FEDERATION_KUBE_CONTEXT:-${FEDERATION_NAME}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just set it to this rather than defaulting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a good idea.

Right now we have a path where FEDERATION_KUBE_CONTEXT and FEDERATION_NAME might not be the same. The plan is to remove that path in 1.7. I will make the recommended change in the PR that removes the alternative path.

@fejta
Copy link
Contributor

fejta commented Apr 7, 2017

/lgtm

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fejta, madhusudancs, shashidharatd

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-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 7, 2017
@madhusudancs madhusudancs removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Apr 7, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 44191, 44117, 44072)

@k8s-github-robot k8s-github-robot merged commit 8c61829 into kubernetes:master Apr 7, 2017
This was referenced Apr 7, 2017
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fejta fejta fejta left review comments

Assignees

@madhusudancs madhusudancs

@fejta fejta

@shashidharatd shashidharatd

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@madhusudancs @k8s-reviewable @shashidharatd @fejta @k8s-github-robot @k8s-ci-robot