Skip to content
This repository was archived by the owner on Feb 24, 2020. It is now read-only.

Conversation

@euank
Copy link
Member

@euank euank commented Nov 22, 2016

This change reuses the image list flag parsing for list.

It also makes list and list --format=json have more similar behavior
in when the pod manifest is skipped.

Fixes #3402

I'd also like to add more tests since I wouldn't be surprised if the coverage is a bit low here.

Copy link
Member

@lucab lucab left a comment

Choose a reason for hiding this comment

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

LGTM, just a minor request to preserve one of the original comments. Feel free to self-merge when done.

if err == nil {
return t, nil
}
if !os.IsNotExist(err) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you please retain the original exited-without-pid comment here?

@lucab lucab added this to the v1.20.0 milestone Nov 23, 2016
@lucab lucab self-assigned this Nov 23, 2016
This change reuses the `image list` flag parsing for `list`.

It also makes `list` and `list --format=json` have more similar behavior
in when the pod manifest is skipped.

Fixes rkt#3402
@euank euank force-pushed the fix-misc-list-issues branch from 65b3511 to 2698e6c Compare November 23, 2016 18:43
@euank
Copy link
Member Author

euank commented Nov 23, 2016

Comment addressed, the semaphore failure is the kvm rkt stop one.

@lucab
Copy link
Member

lucab commented Nov 23, 2016

Thanks, looks nice. I'm pretty much convinced that the kvm-stop failures are due to #3382, should we perhaps disable the test in the meanwhile to not impact other PRs?

@euank
Copy link
Member Author

euank commented Nov 23, 2016

If we're going to only ignore it and it flakes/fails this badly, yeah.

@s-urbaniak s-urbaniak merged commit fce0966 into rkt:master Nov 24, 2016
@euank euank deleted the fix-misc-list-issues branch November 27, 2016 03:13
euank added a commit to euank/rktlet that referenced this pull request Nov 28, 2016
See rkt/rkt#3403

This changes rktlet to re-use the type exported there and updates for
the size type-change as well.
euank added a commit to euank/kubernetes that referenced this pull request Jan 20, 2017
See rkt/rkt#3403

This changes rktlet to re-use the type exported there and updates for
the size type-change as well.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants