Add a script to measure the execution times of podman, crun, run and
conmon. It's a trimmed down version of the exitsnoop tool and intended
to guide us in future performance optimizations.
The below output was generated when running
`podman run --net=host docker.io/library/alpine:latest true`
```
podman (snoop) $ sudo ./hack/podmansnoop
PCOMM PID PPID TID AGE(ms)
conmon 51580 51569 51580 1.67
conmon 51583 51569 51583 3.53
crun 51591 51590 51591 18.28
crun 51593 51569 51593 2.48
conmon 51606 51594 51606 0.85
crun 51608 51594 51608 2.50
podman 51594 51590 51594 176.27
conmon 51590 1950 51590 214.78
podman 51569 40964 51569 431.36
```
In the future, it would be helpful to add the arguments of the commands.
`execsnoop` can reveal them quite nicely but I did not manage to merge
the two scripts due to time constraints.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
In each options/foo.md, keep a list of where the option is used.
This will be valuable to anyone making future edits, and to
those reviewing those edits.
This may be a controversial commit, because those crossref lists
are autogenerated as a side effect of the script that reads them.
It definitely violates POLA. And one day, some kind person will
reconcile (e.g.) --label, using it in more man pages, and maybe
forget to git-commit the rewritten file, and CI will fail.
I think this is a tough tradeoff, but worth doing. Without this,
it's much too easy for someone to change an option file in a way
that renders it inapplicable/misleading for some podman commands.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Support swagger testing and optional runtime updates similar to
the current golangci-lint tool. This allows developers to update the
version of swagger at runtime if needed. Otherwise new CI VM images
will pick up the prescribed version at image build-time via
`make install.tools`.
Signed-off-by: Chris Evich <cevich@redhat.com>
Print the diff at the end of the report to help better understand what's
going on.
```
tree is dirty, please run "make vendor" and commit all changes.
M go.mod
M go.sum
M hack/tree_status.sh
M vendor/github.com/containers/storage/store.go
M vendor/modules.txt
---------------------- Diff below ----------------------
diff --git a/go.mod b/go.mod
index e36d3fb95c57..167d769c378f 100644
--- a/go.mod
+++ b/go.mod
@@ -17,7 +17,7 @@ require (
github.com/containers/image/v5 v5.23.0
github.com/containers/ocicrypt v1.1.6
github.com/containers/psgo v1.7.3
- github.com/containers/storage v1.43.1-0.20221013143630-714f4fc6e80e
+ github.com/containers/storage v1.43.1-0.20221014072257-a144fee6f51c
github.com/coreos/go-systemd/v22 v22.4.0
github.com/coreos/stream-metadata-go v0.0.0-20210225230131-70edb9eb47b3
github.com/cyphar/filepath-securejoin v0.2.3
```
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
This is what was supposed to be an easy two-or-three-line
change to enable a more general-purpose include mechanism
than '@@option'; one that could include an arbitrary file.
This is commit 2 of 2, the "easy" part. Unfortunately, it's
not looking good. The source .md file has UTF8 checkmarks,
and nroff is not happy with those: the generated man pages
are gross.
Another problem: the source .md might need tweaking, because
we don't want a level 1 header in the man page. Obvious solution
is to make kubernetes_support.md a .md.in file as well, and
move the tables to a separate file (or files). Deferred for later.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Refactoring needed in order to add a more general-purpose
include mechanism. Functionality remains the same, and
oh, how I've tested! Unfortunately it's not possible to
review this, at least, not via diffs. Should you be
inclined to review, you'll need to treat it as a
completely brand-new script and test.
This is commit 1 of 2: basically, retain 100% compatibility
with what we have at the moment. Commit 2 will add the
new include mechanism. That one is easy to review.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Reintroduce .install.foo targets into Makefile, and invoke
only the bare-minimum ones needed for each individual CI
step in setup_environment.sh.
Also add a retry to the golangci-lint curl, in hopes of
dealing with network flakes. And remove the -f (fail)
because it produces unhelpful logs.
Reason: saw about 25% CI flakes yesterday due to the golangci-lint
fetch, something about a timeout, and this was especially frustrating
because none of the steps actually needed lint. Quick reminder:
avoid network fetches unless absolutely necessary.
Fixes: #15892
Signed-off-by: Ed Santiago <santiago@redhat.com>
Issue #15923 should have never happened: the problem should've
been autodetected. Make it so henceforth (and fix another
existing discrepancy)
Signed-off-by: Ed Santiago <santiago@redhat.com>
Followup to #15616, which is not usable as it is (way, way, way
too much noise) but actually found a few real nits that should
be fixed.
Signed-off-by: Ed Santiago <santiago@redhat.com>
...gathered up from the last few months of almost-daily runs.
The principal difference is, ditching the git-am approach in
favor of git-cherry-pick. It's so much nicer! I keep forgetting
how clumsy git-am is. With the new approach, saved checkpoints
are kept as git branches, not in an easy-to-lose text file.
And, conflict resolution is MUCH EASIER. (Conflict resolution
is necessary when, e.g., the treadmill PR includes fixes for
some new vendoring that buildah has done but not podman, then
podman vendors in that same module but fixes broken tests in
a different way than I did).
Also a lot of smaller fixes for bugs reported by @Luap99.
Thank you for testing and for letting me know of problems!
Cursory review is OK: this will not break anything in the repo,
and I've been testing/finetuning these changes heavily over
the past month or two.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Podman adds an Error: to every error message. So starting an error
message with "error" ends up being reported to the user as
Error: error ...
This patch removes the stutter.
Also ioutil.ReadFile errors report the Path, so wrapping the err message
with the path causes a stutter.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
This one is a nightmare, because --volume has been edited
in four different files throughout the years (five if you
count podman-build, which I am not including in this PR).
Those edits have not always been done in sync.
The list of options was reordered 2022-06-28 by Giuseppe in #14734,
but only in podman-create and -run (not in podman-pod-*). No
explanation of why, but I'll assume he knew what he was doing,
and have accepted that for the reference copy.
There was also a big edit in #8519.
The "Propagation property...bind mounted" sentence first appeared
in pod-clone, in #14299 by cdoern, with no obvious source of where
it came from. I choose to include it in the reference copy.
The "**copy**" option seems to work in pod-create, so I'm including
it in the reference copy. Someone please yell loudly if this is
not the case.
The "disables SELinux separation for containers used in the build",
no idea, changed that to just "for the container/pod"
The "advanced users / overlay / upperdir / workdir" paragraph
makes zero sense to me, but hey, I assume it applies to all
the commands, so I put it in the reference copy.
Finally, there's still a mishmash of backticks, asterisks, underscores,
and even quotation marks. Someone is gonna have to perform major
cleanup on this one day, but at least it'll be in only one place.
Signed-off-by: Ed Santiago <santiago@redhat.com>
...and, tweak markdown-process-review so it can detect and
remove identical files, making review easier.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Smaller, more reviewable chunks.
This is just one option, --arch. Future PRs may, if the reviewing
is easy, include multiple options. This one includes fixes to
the preprocessor script, though:
* big oops, I was not handling '<<something pod|something>>'
where 'pod' appears other than the beginning of the string.
* I was also not handling 'container<<| or pod>>', where one
side was empty.
* Behavior change: <<subcommand>>, on podman-pod-foo,
becomes just 'foo' (not 'pod foo'). This will be useful
in a future PR where we refactor --pod-id-file.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Followup to #15174. These are the options that are easy(ish)
to review: those that have only drifted slightly, and need
only minor tweaks to bring back to sanity. For the most part,
I went with the text in podman-run because that was cleaned up
in #5192 way back in 2020. These diffs primarily consist of
using '**' (star star) instead of backticks, plus other
formatting and punctuation changes.
This PR also adds a README in the options dir, and a new
convention: <<container text...|pod text...>> which tries
to do the right thing based on whether the man page name
includes "-pod-" or not. Since that's kind of hairy code,
I've also added a test suite for it.
Finally, since this is impossible to review by normal means,
I'm temporarily committing hack/markdown-preprocess-review,
a script that will diff option-by-option. I will remove it
once we finish this cleanup, but be advised that there are
still 130+ options left to examine, and some of those are
going to be really hard to reunite.
Review script usage: simply run it (you need to have 'diffuse'
installed). It isn't exactly obvious, but it shouldn't take more
than a minute to figure out. The rightmost column (zzz-chosen.md)
is the "winner", the actual content that will be used henceforth.
You really want an ultrawide screen here.
Signed-off-by: Ed Santiago <santiago@redhat.com>
go-md2man happily ignores our comment lines in most cases,
but sphinx (used in readthedocs) cannot deal with comments
if they immediately follow any other content line:
blah blah
[//]: # (my comment)
...the whole comment line is actually rendered in its output.
Only solution seems to be to add extra newlines before each
comment. Makes diff and PR review harder, but otherwise has
no effect on the rendered documents.
Signed-off-by: Ed Santiago <santiago@redhat.com>
str.removeprefix() and str.removesuffix() is python 3.9+ only but we need to
support older versions for the OSX cross task.
This fixes broken CI on main.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
podman-create and -run have many options in common. To date,
these are copy-pasted and haphazardly maintained.
Solution: add an include mechanism, '@@option foo', such
that multiple md source files can fetch from one common file.
This is a Phase One commit, a very small subset of what's
possible. Purpose of this commit is ease of review. If this
passes review, much more (trickier stuff) will be forthcoming.
Signed-off-by: Ed Santiago <santiago@redhat.com>
When podman kube play was added the endpoint for the kube play/play kube
commands was switched from the "play kube" endpoint to the new "kube play"
endpoint. This caused issues with the remote client, requiring the need
to use the "play kube" endpoint again in order to avoid these issues.
Signed-off-by: Niall Crowe <nicrowe@redhat.com>
... primarily so that it can support OCI artifacts.
2.8 already seems to exist in the repo.
This requires changing WaitContainerReady to also check
stderr (ultimately because docker/distribution was
updated to a more recent sirupsen/logrus, which logs
by default to stderr instead of stdout).
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... instead of hard-coding a copy of the value.
Notably this makes hack/podman_registry actually
support the documented -i option.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
htpasswd is no longer included in docker.io/library/distribution
after 2.7.0, per https://github.com/docker/distribution-library-image/issues/107 ,
and we want to upgrade to a recent version.
At least system tests currently execute htpasswd from the OS,
so it seems that it is likely to be available.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
While convenient, it can be problematic to rely on a Makefile to install
software. This was found to be the case across multiple environments
WRT `bats`. Fix this by removing the install script and target. A
future commit will ensure the correct version of `bats` is present in
all CI environments where it's required.
Signed-off-by: Chris Evich <cevich@redhat.com>
Rename all files to _test.go and rename the package to e2e_test. This
makes the linter less strict about things like dot imports.
Add some unused nolint directives to silence some warnings, these can be
used to find untested options so someone could add tests for them.
Fixes#14996
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Followup to #14906, in which a nonexistent option was found
in a man page. The xref script was designed to catch that,
but I was too lax in my parsing: the option was documented
using wrong syntax, and the script didn't catch it.
Solution: do not allow *any* unrecognized cruft in the
option description lines. And fix all improperly-written
entries to conform to the rule:
**--option**=*value(s)*
Two asterisks around option, which must have two dashes. One
asterisk around value(s).
This is going to cause headaches for some people adding new
options, but I don't think I can fix that: there are many
factors that make an unparseable line. Adding 'hint' code
would make the script even more complex than it is. I have
to assume that our contributors are smart enough to look
at surrounding context and figure out the right way to
specify options.
Signed-off-by: Ed Santiago <santiago@redhat.com>
The "podman kube play" command is designed to be a replacement for the
"podman play kube" command.
It performs the same function as "play kube" while also still working with the same flags and options.
The "podman play kube" command is still functional as an alias of "kube play".
Closes#12475
Signed-off-by: Niall Crowe <nicrowe@redhat.com>
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
manifest_test:authenticated_push() is the final test left to
fix before merging #14397. The reason it's failing _seems_ to be
that podman is running with a mix of netavark and CNI, and
that _seems_ to be because this test invokes hack/podman-registry
which invokes plain podman without whatever options used in e2e.
Starting a registry directly from the test is insane: there is
no reusable code for doing that (see login_logout_test.go and
push_test.go. Yeesh.)
Solution: set $PODMAN, by inspecting the podmanTest object
which includes both a path and a list of options. podman-registry
will invoke that. (It will also override --root and --runroot.
This is the desired behavior).
Also: add cleanup. If auth-push test fails, stop the registry.
Also: add a sanity check to podman-registry script, have it
wait for the registry port to activate. Die if it doesn't.
That could've saved us a nice bit of debugging time.
Signed-off-by: Ed Santiago <santiago@redhat.com>
The golangci installer (which is curl pipe sh, ewww) installs
into $BINDIR, which it gets from the caller's environment.
Make sure we set it explicitly.
Signed-off-by: Ed Santiago <santiago@redhat.com>
We now use the golang error wrapping format specifier `%w` instead of
the deprecated github.com/pkg/errors package.
[NO NEW TESTS NEEDED]
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
Lint the systemd code and fix the reported problems.
The remoteclient tag is no longer used so I just removed it.
[NO NEW TESTS NEEDED]
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Buildah got vendored into podman last week, and the script
went kablooie because of ever-so-slight conflicts between
what was in the treadmill PR (#13808) and what ultimately
got merged (#14127) which was obviously better (hey, I tried).
After a buildah vendor, there really isn't any point to keeping
the treadmill commits - we're much better off just restarting
with two fresh empty placeholder commits. Do so.
Also, mild cleanup.
Signed-off-by: Ed Santiago <santiago@redhat.com>
New script for use with Valentin's benchmarks. Converts ginkgo
timing results to CSV format suitable for (TBI) saving and
comparing.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Add support for new automation library version w/ `$DEBUG` fix
(ref: containers/automation_images#128) and added definitions
for commonly used Distro/version variables.
Signed-off-by: Chris Evich <cevich@redhat.com>
Polish the push and pull benchmarks. In particular, make sure to not be
network bound during these benchmarks by running a local registry and
pushing a local image that can later on be pulled.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
Situation encountered just now after buildah #3949 but
before podman #14084: go.mod changed in such a way that
other modules were updated, not just buildah, and those
changes weren't git-added by 'make vendor'. This resulted
in the dirty-tree CI test failing.
Solution: check for untracked vendor files after 'make vendor',
and git-add them. Show a friendly message that we're doing so:
+---> Adding untracked files under containers/image, containers/storage, klauspost/compress, x/sys
In order to do this safely, we run an untracked-files check
under vendor as one of the first sanity checks. If there are
any when we start the script, fail early.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Major revamp: instead of stacking a vendor commit on top of
the treadmill changes, do it the other way around: vendor,
then apply treadmill diffs.
Reason: the build-all-new-commits test. Sigh. It fails in the
common case where our treadmill changes include a new struct
element in cmd/podman/images/build.go
Why this is good: well, superficially, it's more intuitive.
Why this is horrible: omg the rebasing games are a nightmare.
When the vendor commit is on top (HEAD), it's ultra-trivial
to drop it, rebase the treadmill changes on main, then add
a new vendor-buildah commit on top. As you can see from the
diffs in this PR, treadmill-as-HEAD introduces all sorts
of complex dance steps in which things can go catastrophically
wrong and you can lose all your treadmill patches. I try very
hard to prevent this, and to offer hints if there's a problem,
and heck in the worst case it's still git so it's still possible
to find lost commits... but it's still much riskier than the
old way.
Alternative I considered: using sed magic to disable the
build-all-new-commits test. So tempting... but that would
also disable the bloat check.
Signed-off-by: Ed Santiago <santiago@redhat.com>
More safety checks for the treadmill script:
* for --sync:
- issue warning if HEAD is not a vendor commit
- if run-buildah-bud-tests fails, leave the working dir
for user to investigate. And offer a long helpful warning.
- tweak .cirrus.yml so buildah-bud tests run early, so
we can fail early. (Remember, the top commit will never
ever ever ever be merged)
* for --pick:
- check branch merge-base (of your vendor-update branch),
compare against that of the treadmill PR. If treadmill
is newer, bail, and suggest rebasing. This would've
saved us some time in #14005.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Add more benchmarks for the most common and performance-critical image
commands. Benchmarks for `podman build` should go into a separate
section.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
This PR introduces a test suite for podman machine. It can currently be
run on developers' local machines and is not part of the official CI
testing; however, the expectation is that any work on machine should
come with an accompanying test.
At present, the test must be run on Linux. It is untested on Darwin.
There is no Makefile target for the test. It can be run like `ginkgo -v
pkg/machine/test/.`. It should be run as a unprivileged user.
Signed-off-by: Brent Baude <bbaude@redhat.com>
