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.

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

Running validation tests from a release #610

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

alban
Copy link
Contributor

@alban alban commented Mar 20, 2018

Document how to do a release including the pre-compiled binaries.

  • docs: update link to spec
  • docs: move compliance testing to separate doc
  • docs: update validation test output
  • docs: how to do a release
  • docs: how to run tests from a release

Fixes #609


- Start at the relevant milestone on GitHub (e.g. https://github.com/opencontainers/runtime-tools/milestones/v0.6.0): ensure all referenced issues are closed (or moved elsewhere, if they're not done). Close the milestone.
- runtime-tools does not use a [roadmap file](https://github.com/opencontainers/runtime-tools/issues/465) but GitHub milestones. Update the [other milestones](https://github.com/opencontainers/runtime-tools/milestones), if necessary
- Branch from the latest master, make sure your git status is clean
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want backticks for git status.

Also, some of these list entries end in periods (e.g. “Close the milestone.”) and some don't (e.g. this one). I think we should pick whether we want list entries to include trailing punctuation and then be consistent about it. My personal preference is for trailing punctuation, but I'm fine with either approach as long as we're consistent.

[maintainers]: https://github.com/opencontainers/runtime-tools/blob/master/MAINTAINERS
[mailinglist]: https://groups.google.com/a/opencontainers.org/forum/#!forum/dev
[gh-new-release]: https://github.com/opencontainers/runtime-tools/releases/new

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this trailing blank line.

Ensure the branch is correct:

- Ensure the build is clean!
- `git clean -ffdx && make && make test` should work
Copy link
Contributor

Choose a reason for hiding this comment

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

Which OS/arch(es) are we building for? Just GOOS=linux GOARCH=amd64? Do we want to build for each of the OSes listed here? For some set of arches?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally, all, but I don't know how to do that. Could we start with linux + amd64 in this PR and improve later based on contributions? But we should at least namespace the filename of the tarball correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we start with linux + amd64 in this PR and improve later based on contributions?

That's fine with me, but ideally these instructions are the same for all platforms. I think the missing piece is a build farm supporting all of our target platforms. Are we waiting for Windows and Solaris folks to point us at farms for their platforms?

- File a PR
- Ensure the CI on the release PR is green
- Send an email to the [mailing list][mailinglist] ([example for v0.5.0](https://groups.google.com/a/opencontainers.org/forum/#!topic/dev/iuWpWUai4_I)) and get reviews from other [maintainers][maintainers].
- Once the maintainers agree, merge the PR
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we land local copies of the governance docs that we can link here to expand on “agree”? My last attempt at that was #274, but perhaps the maintainers have a different plan in mind for maintaining governance docs for this project.


```
$ tar xf runtime-tools-v0.6.0.tar.gz
$ RUNTIME=runc tap ./runtime-tools-v0.6.0/validation/*.t
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be consistent about whether or not we recommend using sudo for the tests. In the “From source” section below, we recommend using sudo, so I think we want to also recommend it here.


Check if your release has pre-compiled tests on the [release page][releases] page.

```
Copy link
Contributor

Choose a reason for hiding this comment

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

You want:

```console

here.

- Attach the release.
This is a simple tarball:

```
Copy link
Contributor

Choose a reason for hiding this comment

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

You want:

```sh

here. Or maybe we should add $ prompts and use:

```console

to be consistent with our other examples.

- Clean your git tree: `sudo git clean -ffdx`.

[changelog]: https://github.com/opencontainers/runtime-tools/blob/master/CHANGELOG.md
[maintainers]: https://github.com/opencontainers/runtime-tools/blob/master/MAINTAINERS
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 prefer if these were relative links, e.g. [mantainers]: ../../MANTAINERS, to decouple them from forking and other hosting changes. We can't decouple all of our links from the host URI (e.g. for releases below), but I'd like relative links where the target is also in this Git repository.

$ make runtimetest validation-executables
```

Runtime validation currently [only supports](docs/runtime-compliance-testing.md) the [OCI Runtime Command Line Interface](doc/command-line-interface.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

These relative links (and some later ones in this file) need to be updated now that you've moved the text under docs/.

Also, this information is generic for runtime-validation; it's not specific to the “From source” approach. Can you shift it (and some of the text from the next paragraph) up into the “Running the runtime validation suite” section?

Result: PASS
```


Copy link
Contributor

Choose a reason for hiding this comment

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

You only need one blank line here.

@alban
Copy link
Contributor Author

alban commented Mar 21, 2018

@wking thanks for the review! I'll fix that after I finish my current task (test for namespace path).

runtime-spec v1.0.1 is released. No need to link to a pre-v1 rc anymore.

Signed-off-by: Alban Crequy <[email protected]>
Signed-off-by: Alban Crequy <[email protected]>
@dongsupark dongsupark force-pushed the alban/release branch 3 times, most recently from 7203f73 to a55083e Compare August 22, 2018 14:50
@dongsupark
Copy link
Contributor

Rebased, and addressed review comments by @wking.

README.md Outdated
Files=1, Tests=1, 0 wallclock secs ( 0.01 usr 0.01 sys + 0.03 cusr 0.03 csys = 0.08 CPU)
Result: PASS
```
Use the [runtime validation suite](doc/runtime-compliance-testing.md).

Choose a reason for hiding this comment

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

doc -> docs

Copy link
Contributor

Choose a reason for hiding this comment

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

@q384566678 Good catch. Fixed. Thanks!

Address review comments written by @wking.

* Add a blank line between non-manpage refs and manpage refs.
* Add backticks for `git status`.
* Append a punctuation mark to each list item.
* Use `pull request` instead of `PR`.
* Specify `console` format to the example commands
* Use relative links instead of static URIs
* Remove a trailing blank
* Remove trailing whitespaces
* Run with sudo whenever it's possible
* Use correct relative links
* Move sections for running runtime validation one level up to the
  section `Running the runtime validation suite`.
* Use `command -v` instead of `which`.
* Elaborate why we should specify the TAP variable.

Signed-off-by: Dongsu Park <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants