Skip to content

Conversation

@cgwalters
Copy link
Member

Drain another thing out of cmd-run. This is prep for making
cmd-run be a tiny wrapper around kola qemuexec.

Also, using 9p mounts can be very useful for testing; i.e. this
helps unlock having qemu-specific tests that bind mount in data
without copying.

(One thing I'm thinking for example is that we support pulling
container images from the host)

@cgwalters
Copy link
Member Author

Depends #1284

@jlebon
Copy link
Member

jlebon commented Mar 25, 2020

All these draining PRs are great, though it'd be good to discuss a bit what the end goal is. In my mind, I see e.g. cmd-kola and cmd-run as becoming just compat shorthands for kola and kola qemuexec respectively (and eventually just dropping them). Or do you see them as continuing to wrap some lower-level switches?

I like the trend of making kola easier to use and just going for that directly. For example, it Just Works now to kola spawn in a cosa workdir.

So here for example, if we go that way, we could teach kola that --srv switch as a shorthand for --bind-rw=... and drop it entirely from cmd-run.

@cgwalters
Copy link
Member Author

I think we do want to get rid of the wrappers eventually, but I'm trying to preserve the existing UX to some degree because...well I use cosa run a lot and I'd have to retrain my fingers.

@cgwalters
Copy link
Member Author

So here for example, if we go that way, we could teach kola that --srv switch as a shorthand for --bind-rw=... and drop it entirely from cmd-run.

That one makes sense yeah.

@jlebon
Copy link
Member

jlebon commented Mar 27, 2020

Yup, much nicer! LGTM once rebased on top of #1291.
/approve

Drain another thing out of `cmd-run`.  This is prep for making
`cmd-run` be a tiny wrapper around `kola qemuexec`.

Also, using 9p mounts can be very useful for testing; i.e. this
helps unlock having qemu-specific tests that bind mount in data
without copying.

(One thing I'm thinking for example is that we support pulling
container images from the host)
@cgwalters
Copy link
Member Author

Rebased 🏄‍♂️

@jlebon
Copy link
Member

jlebon commented Mar 27, 2020

/lgtm

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jlebon

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-merge-robot openshift-merge-robot merged commit 395f65e into coreos:master Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants