cp exec util from kubernetes/kubernetes by mengqiy · Pull Request #5 · kubernetes/utils

mengqiy · 2017-07-07T21:11:06Z

No description provided.

…hey're in $PATH on all.

Use %v for errors, tidy some messages, make error messages start lowe-case (as per go guidelines). Just accumulated nits.

Instead of saying "Google Inc." (which is not always correct) say "The Kubernetes Authors", which is generic.

Auto commit by PR queue bot

Also add Output() to the util/exec Cmd interface.

- improve restoreInternal implementation in iptables - add SetStdin and SetStdout functions to Cmd interface - modify kubelet/prober and some tests in order to work with Cmd interface

Add canonical imports only in existing doc.go files. https://golang.org/doc/go1.4#canonicalimports Fixes #29014

Automatic merge from submit-queue improve iptables-restore implementation #27559 [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]() fixes #27559 - improve restoreInternal implementation in iptables - add SetStdin and SetStdout functions to Cmd interface - modify kubelet/prober and some tests in order to work with Cmd interface

`ex.Command()` already searches the binary in PATH, no need to manually specify it. `pkg/util/exec` tests fail in non-conventional environments due to this (e.g. NixOS).

mengqiy · 2017-07-07T21:18:58Z

Travis finally works.

apelisse · 2017-07-07T21:25:17Z

@@ -0,0 +1,18 @@
+/*
+Copyright 201i7 The Kubernetes Authors.


Did you make that typo?

We need to be able to verify license in files :-)

we use this file in k/k: https://github.com/kubernetes/kubernetes/blob/master/hack/verify-boilerplate.sh

spxtr · 2017-07-14T18:51:35Z

The CLA won't pass since this includes commits made before we switched to CNCF CLA. That's fine, merge over it.

spxtr · 2017-07-14T18:56:41Z

Not sure why travis isn't responding.

apelisse · 2017-07-18T16:59:10Z

It's running!

apelisse · 2017-07-18T17:00:17Z

Yay, now we know it's broken ;-)

mengqiy · 2017-07-18T17:27:18Z

It was broken because I move fake_exec.go to test/. Now it has been fixed. It's passing now :)

mengqiy · 2017-07-18T20:37:52Z

We need to wait till this PR get merged first.
Then we can send a PR in the main repo to update the vendor directory.

apelisse

Thanks Mengqi!

apelisse · 2017-07-18T23:41:18Z

+limitations under the License.
+*/
+
+package exec


Should this be named package exec even though it's actually in exec/testing/?

Changed to testingexec.

apelisse · 2017-07-18T23:42:16Z

+
+// Wraps exec.Cmd so we can capture errors.
+type cmdWrapper osexec.Cmd
+


Does it make sense to add

var _ Cmd = &cmdWrapper{}

Done. It's good to have a check to make sure it has implemented the interface.
Also add the check in testing/fake_exec.go. Does it make sense?

Yes I think it does. I didn't mention it because func InitFakeCmd(fake *FakeCmd, cmd string, args ...string) exec.Cmd would fail if fake wasn't a exec.Cmd, but that's probably more subtle that necessary. Thanks for the changes!

apelisse · 2017-07-18T23:42:53Z

+// ErrExecutableNotFound is returned if the executable is not found.
+var ErrExecutableNotFound = osexec.ErrNotFound
+
+// Interface is an interface that presents a subset of the os/exec API.  Use this


Very dumb nit: double space after period

apelisse · 2017-07-18T23:42:59Z

+}
+
+// Cmd is an interface that presents an API that is very similar to Cmd from os/exec.
+// As more functionality is needed, this can grow.  Since Cmd is a struct, we will have


Very dumb nit: double space after period

apelisse · 2017-07-18T23:43:03Z

+	// Run runs the command to the completion.
+	Run() error
+	// CombinedOutput runs the command and returns its combined standard output
+	// and standard error.  This follows the pattern of package os/exec.


Very dumb nit: double space after period

apelisse · 2017-07-18T23:43:10Z

+}
+
+// ExitError is an interface that presents an API similar to os.ProcessState, which is
+// what ExitError from os/exec is.  This is designed to make testing a bit easier and


Very dumb nit: double space after period

mengqiy · 2017-07-19T00:32:34Z

@apelisse PTAL
I will squash before merging.

apelisse

lgtm!

apelisse · 2017-07-19T03:10:57Z

+
+// Wraps exec.Cmd so we can capture errors.
+type cmdWrapper osexec.Cmd
+


Yes I think it does. I didn't mention it because func InitFakeCmd(fake *FakeCmd, cmd string, args ...string) exec.Cmd would fail if fake wasn't a exec.Cmd, but that's probably more subtle that necessary. Thanks for the changes!

apelisse · 2017-07-19T03:11:46Z

Forced the merge because of cla errors.

mengqiy · 2017-07-19T22:00:07Z

Associated PR against main repo: kubernetes/kubernetes#49234

@apelisse

Automatic merge from submit-queue (batch tested with PRs 49107, 47177, 49234, 49224, 49227) Move util/exec to vendor Move util/exec to vendor. Update import paths. Update godep Part of #48209 Associate PR against `k8s.io/utils` repo: kubernetes/utils#5 ```release-note NONE ``` /assign @apelisse

@apelisse

Automatic merge from submit-queue (batch tested with PRs 49107, 47177, 49234, 49224, 49227) Move util/exec to vendor Move util/exec to vendor. Update import paths. Update godep Part of #48209 Associate PR against `k8s.io/utils` repo: kubernetes/utils#5 ```release-note NONE ``` /assign @apelisse

@apelisse

Automatic merge from submit-queue (batch tested with PRs 49107, 47177, 49234, 49224, 49227) Move util/exec to vendor Move util/exec to vendor. Update import paths. Update godep Part of #48209 Associate PR against `k8s.io/utils` repo: kubernetes/utils#5 ```release-note NONE ``` /assign @apelisse

@apelisse

Automatic merge from submit-queue (batch tested with PRs 49107, 47177, 49234, 49224, 49227) Move util/exec to vendor Move util/exec to vendor. Update import paths. Update godep Part of #48209 Associate PR against `k8s.io/utils` repo: kubernetes#5 ```release-note NONE ``` /assign @apelisse

@apelisse

Automatic merge from submit-queue (batch tested with PRs 49107, 47177, 49234, 49224, 49227) Move util/exec to vendor Move util/exec to vendor. Update import paths. Update godep Part of kubernetes#48209 Associate PR against `k8s.io/utils` repo: kubernetes/utils#5 ```release-note NONE ``` /assign @apelisse

thockin and others added 24 commits October 6, 2014 16:52

Add a util/exec interface for testing execs.

83db8cb

Fix the exec test, on os x false and true have different paths, but t…

89df886

…hey're in $PATH on all.

Add support for git volumes.

a239f43

Clean up error logs.

3bd48a7

Use %v for errors, tidy some messages, make error messages start lowe-case (as per go guidelines). Just accumulated nits.

Make copyright ownership statement generic

89cef67

Instead of saying "Google Inc." (which is not always correct) say "The Kubernetes Authors", which is generic.

pkg/util/exec: allow mocking of LookPath

44277a3

Print unhandeled command in FakeExec

ad86408

Handle executable not found in exec

c932095

Merge pull request #16950 from swagiaal/handle-exec-not-found

db0e323

Auto commit by PR queue bot

Improve error reporting a little in ingress e2e.

39529cf

Also add Output() to the util/exec Cmd interface.

improve iptables-restore implementation #27559

9dd79d6

- improve restoreInternal implementation in iptables - add SetStdin and SetStdout functions to Cmd interface - modify kubelet/prober and some tests in order to work with Cmd interface

Use Go canonical import paths

fed5729

Add canonical imports only in existing doc.go files. https://golang.org/doc/go1.4#canonicalimports Fixes #29014

pkg/util/exec: don't specify paths for echo

a66ca68

`ex.Command()` already searches the binary in PATH, no need to manually specify it. `pkg/util/exec` tests fail in non-conventional environments due to this (e.g. NixOS).

Rectify kubectl error output

9589c6e

Add return code support to kubectl-exec and -run

6122caf

autogenerated

966490d

autoupdate BUILD files

34f72a5

autogenerated

d98787a

Enable auto-generating sources rules

7799517

Add support for attacher/detacher interface in Flex volume

3be53a2

Extend Iptables interface with SaveInto

0171d94

Remove Save() from iptables interface

de2161e

mengqiy assigned apelisse and thockin Jul 7, 2017

apelisse reviewed Jul 7, 2017

View reviewed changes

mengqiy force-pushed the util_exec branch from 4c71b23 to d20ccd1 Compare July 7, 2017 21:28

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 7, 2017

spxtr closed this Jul 14, 2017

spxtr reopened this Jul 14, 2017

mengqiy force-pushed the util_exec branch from 1bf950b to af04c25 Compare July 17, 2017 20:10

apelisse closed this Jul 18, 2017

apelisse reopened this Jul 18, 2017

update pkg info and structure.

c7bfd26

mengqiy force-pushed the util_exec branch from af04c25 to c7bfd26 Compare July 18, 2017 17:24

mengqiy mentioned this pull request Jul 18, 2017

pkgs to be moved to utils repo #9

Open

1 of 4 tasks

apelisse reviewed Jul 18, 2017

View reviewed changes

address comments

1641070

mengqiy force-pushed the util_exec branch from d67b248 to 1641070 Compare July 19, 2017 00:28

apelisse approved these changes Jul 19, 2017

View reviewed changes

apelisse merged commit 9fdc871 into kubernetes:master Jul 19, 2017

mengqiy deleted the util_exec branch July 19, 2017 03:27

mengqiy mentioned this pull request Jul 19, 2017

Move util/exec to vendor kubernetes/kubernetes#49234

Merged

nktpro mentioned this pull request Feb 28, 2020

[mount] Addition of "checkAndRepairXfsFilesystem" inadvertently prevents XFS self-recovery via mounting #141

Closed


		// Wraps exec.Cmd so we can capture errors.
		type cmdWrapper osexec.Cmd

Conversation

mengqiy commented Jul 7, 2017

Uh oh!

mengqiy commented Jul 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spxtr commented Jul 14, 2017

Uh oh!

spxtr commented Jul 14, 2017

Uh oh!

apelisse commented Jul 18, 2017

Uh oh!

apelisse commented Jul 18, 2017

Uh oh!

mengqiy commented Jul 18, 2017

Uh oh!

mengqiy commented Jul 18, 2017

Uh oh!

apelisse left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mengqiy commented Jul 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apelisse left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apelisse commented Jul 19, 2017

Uh oh!

mengqiy commented Jul 19, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

mengqiy commented Jul 19, 2017 •

edited

Loading