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

Check if netstat or iproute2 is available #59852

Merged
merged 1 commit into from
Feb 15, 2018
Merged

Check if netstat or iproute2 is available #59852

merged 1 commit into from
Feb 15, 2018
+11 −2

Conversation

cblecker
Copy link
Member

@cblecker cblecker commented Feb 14, 2018 •
edited
Loading

What this PR does / why we need it:
In #59655, I incorrectly assumed that the kubetest image would have net-tools. It doesn't. This checks and uses ss from iproute2, or netstat from net-tools. The kubetest image does appear to have iproute2.

Release note:

NONE

@cblecker cblecker added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 14, 2018
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 14, 2018
@cblecker cblecker mentioned this pull request Feb 14, 2018
Merged
@dims
Copy link
Member

dims commented Feb 14, 2018

@cblecker did you mean to push the pipe error message commit to this PR?

stevekuznetsov
stevekuznetsov reviewed Feb 14, 2018
View reviewed changes
hack/lib/etcd.sh Outdated
@@ -28,7 +28,16 @@ kube::etcd::validate() {
}

# validate etcd port is free
if netstat -nat | grep "[\.:]${ETCD_PORT:?} .*LISTEN" >/dev/null 2>&1; then
local port_check_command
if which ss &> /dev/null && ss -Version | grep 'iproute2' &> /dev/null; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use command -v as it's apparently kosher for implementations of which to not set exit status and look at loads of things other than installed binaries. On my machine, which is:

$ type which
which is aliased to `(alias; declare -f) | /usr/bin/which --tty-only --read-alias --read-functions --show-tilde --show-dot'

@cblecker cblecker force-pushed the etcd-fix branch 2 times, most recently from 3d51213 to ede8e0a Compare February 14, 2018 18:32
@cblecker
Copy link
Member Author

/hold cancel

PTAL @dims @stevekuznetsov :)

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 14, 2018
@stevekuznetsov
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2018
@stevekuznetsov
Copy link
Contributor

/hold

for @dims

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 14, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, stevekuznetsov

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

The pull request process is described here

Needs approval from an approver in each of these OWNERS Files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dims
Copy link
Member

dims commented Feb 14, 2018

/hold cancel

LGTM 👍

ubuntu@testvm-1:~/go/src/k8s.io/kubernetes$ export ETCD_PORT=2379
ubuntu@testvm-1:~/go/src/k8s.io/kubernetes$ ss -nat | grep "LISTEN" | grep "[\.:]${ETCD_PORT:?}"
LISTEN     0      128    127.0.0.1:2379                     *:*
ubuntu@testvm-1:~/go/src/k8s.io/kubernetes$ netstat -nat | grep "LISTEN" | grep "[\.:]${ETCD_PORT:?}"
tcp        0      0 127.0.0.1:2379          0.0.0.0:*               LISTEN

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 14, 2018
@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 59878, 59852). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 571158e into kubernetes:master Feb 15, 2018
@cblecker cblecker deleted the etcd-fix branch February 15, 2018 06:56
@wojtek-t wojtek-t mentioned this pull request Apr 5, 2018
Closed
@nikhita nikhita mentioned this pull request Jan 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevekuznetsov stevekuznetsov stevekuznetsov left review comments

@dims dims dims approved these changes

Assignees

@stevekuznetsov stevekuznetsov

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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cblecker @dims @stevekuznetsov @k8s-ci-robot @k8s-github-robot