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.

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

Adding option to set the federation api server port if nodeport is set #46283

Merged
merged 5 commits into from Jul 30, 2017

Conversation

@ktsakalozos
Collaborator

ktsakalozos commented May 23, 2017

What this PR does / why we need it: Kubefed will deploy the respected services and then it will do a health check. Prior to this patch if the user selects the nodeport a random port is opened. In environments where firewalls are in place this random port selection will cause the health check to fail. With this patch we enable users to designate a specific port, after for example opening it on their firewall.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #46021

Special notes for your reviewer:

Release note:

Kubefed init allows for setting port in Nodeport configuration
@k8s-ci-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-ci-robot

k8s-ci-robot May 23, 2017

Contributor

Hi @ktsakalozos. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Contributor

k8s-ci-robot commented May 23, 2017

Hi @ktsakalozos. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Show outdated Hide outdated federation/pkg/kubefed/init/init.go Outdated
Show outdated Hide outdated federation/pkg/kubefed/init/init.go Outdated
Show outdated Hide outdated federation/pkg/kubefed/init/init.go Outdated
Show outdated Hide outdated federation/pkg/kubefed/init/init.go Outdated
Show outdated Hide outdated federation/pkg/kubefed/init/init.go Outdated
Show outdated Hide outdated federation/pkg/kubefed/init/init.go Outdated
@nikhiljindal

This comment has been minimized.

Show comment
Hide comment
@nikhiljindal

nikhiljindal Jun 5, 2017

Member

@k8s-bot ok to test

Member

nikhiljindal commented Jun 5, 2017

@k8s-bot ok to test

ktsakalozos added some commits Jun 6, 2017

@ktsakalozos

This comment has been minimized.

Show comment
Hide comment
@ktsakalozos

ktsakalozos Jun 13, 2017

Collaborator

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

Collaborator

ktsakalozos commented Jun 13, 2017

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

@perotinus

I'm on the fence about whether you should add a test or not: the tests for init are somewhat cumbersome, but regardless it's probably good to have a test for this flag.

@perotinus

This comment has been minimized.

Show comment
Hide comment
@perotinus

perotinus Jun 20, 2017

Member

The code changes here LGTM, but I can't give OWNERs approval.

Member

perotinus commented Jun 20, 2017

The code changes here LGTM, but I can't give OWNERs approval.

@nikhiljindal

This comment has been minimized.

Show comment
Hide comment
@nikhiljindal

nikhiljindal Jul 27, 2017

Member

/lgtm
/approve

Approving based on @perotinus's review

Member

nikhiljindal commented Jul 27, 2017

/lgtm
/approve

Approving based on @perotinus's review

@k8s-merge-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-merge-robot

k8s-merge-robot Jul 27, 2017

Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ktsakalozos, nikhiljindal

Associated issue: 46021

The full list of commands accepted by this bot can be found here.

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

Contributor

k8s-merge-robot commented Jul 27, 2017

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ktsakalozos, nikhiljindal

Associated issue: 46021

The full list of commands accepted by this bot can be found here.

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

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 28, 2017

/retest
Automated flake /retester experiment. Please send feedback to fejta

fejta-bot commented Jul 28, 2017

/retest
Automated flake /retester experiment. Please send feedback to fejta

@ktsakalozos

This comment has been minimized.

Show comment
Hide comment
@ktsakalozos

ktsakalozos Jul 28, 2017

Collaborator

/test pull-kubernetes-federation-e2e-gce
/test pull-kubernetes-e2e-kops-aws

Collaborator

ktsakalozos commented Jul 28, 2017

/test pull-kubernetes-federation-e2e-gce
/test pull-kubernetes-e2e-kops-aws

@ktsakalozos

This comment has been minimized.

Show comment
Hide comment
@ktsakalozos

ktsakalozos Jul 28, 2017

Collaborator

/test pull-kubernetes-e2e-kops-aws

Collaborator

ktsakalozos commented Jul 28, 2017

/test pull-kubernetes-e2e-kops-aws

2 similar comments
@ktsakalozos

This comment has been minimized.

Show comment
Hide comment
@ktsakalozos

ktsakalozos Jul 28, 2017

Collaborator

/test pull-kubernetes-e2e-kops-aws

Collaborator

ktsakalozos commented Jul 28, 2017

/test pull-kubernetes-e2e-kops-aws

@ktsakalozos

This comment has been minimized.

Show comment
Hide comment
@ktsakalozos

ktsakalozos Jul 28, 2017

Collaborator

/test pull-kubernetes-e2e-kops-aws

Collaborator

ktsakalozos commented Jul 28, 2017

/test pull-kubernetes-e2e-kops-aws

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

fejta-bot commented Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

1 similar comment
@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

fejta-bot commented Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

@xiangpengzhao

This comment has been minimized.

Show comment
Hide comment
@xiangpengzhao

xiangpengzhao Jul 29, 2017

Member

/retest
Testing this flake: #49824

Member

xiangpengzhao commented Jul 29, 2017

/retest
Testing this flake: #49824

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

fejta-bot commented Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

@ktsakalozos

This comment has been minimized.

Show comment
Hide comment
@ktsakalozos

ktsakalozos Jul 29, 2017

Collaborator

/test pull-kubernetes-bazel

Collaborator

ktsakalozos commented Jul 29, 2017

/test pull-kubernetes-bazel

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

fejta-bot commented Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

2 similar comments
@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

fejta-bot commented Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

fejta-bot commented Jul 29, 2017

/retest
Automatic flake /retester. Please send feedback to @fejta

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 30, 2017

/retest
This /retest bot automatically retries jobs that failed/flaked on approved PRs (send bot feedback to @fejta).

Review the full test history for this PR.

fejta-bot commented Jul 30, 2017

/retest
This /retest bot automatically retries jobs that failed/flaked on approved PRs (send bot feedback to @fejta).

Review the full test history for this PR.

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 30, 2017

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot commented Jul 30, 2017

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

1 similar comment
@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Jul 30, 2017

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot commented Jul 30, 2017

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

@k8s-merge-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-merge-robot

k8s-merge-robot Jul 30, 2017

Contributor

Automatic merge from submit-queue

Contributor

k8s-merge-robot commented Jul 30, 2017

Automatic merge from submit-queue

@k8s-merge-robot k8s-merge-robot merged commit eae2917 into kubernetes:master Jul 30, 2017

9 of 10 checks passed

Submit Queue Required Github CI test is not green: pull-kubernetes-e2e-gce-etcd3
Details
cla/linuxfoundation ktsakalozos authorized
Details
pull-kubernetes-bazel Job succeeded.
Details
pull-kubernetes-e2e-gce-etcd3 Jenkins job succeeded.
Details
pull-kubernetes-e2e-kops-aws Jenkins job succeeded.
Details
pull-kubernetes-federation-e2e-gce Jenkins job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Jenkins job succeeded.
Details
pull-kubernetes-node-e2e Jenkins job succeeded.
Details
pull-kubernetes-unit Jenkins job succeeded.
Details
pull-kubernetes-verify Jenkins job succeeded.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment