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

apiv2 tests: add helpers to start/stop a local registry #9669

Merged
merged 1 commit into from
Mar 10, 2021

Conversation

edsantiago
Copy link
Member

@edsantiago edsantiago commented Mar 8, 2021

...and a rudimentary set of /auth tests for PR#9589 (disabled).

This simply adds a new start_registry() helper function that
allocates a random unused port, pulls a registry image, creates
a local certificate + random username + random password, and
fires everything up. Since none of this is (yet) used in CI,
this is very low risk.

The only infinitessimally-risky change is using a dedicated
subdirectory of $WORKDIR (instead of $WORKDIR itself) as
the podman root. This fixes a dumb oversight on my part:
the workdir has grown to be used for much more than just
podman root; this change removes clutter and makes it
easier for humans to debug in cases of problems.

Signed-off-by: Ed Santiago [email protected]

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 8, 2021
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago

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

The pull request process is described here

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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 8, 2021
...and a rudimentary set of /auth tests for PR#9589 (disabled).

This simply adds a new start_registry() helper function that
allocates a random unused port, pulls a registry image, creates
a local certificate + random username + random password, and
fires everything up. Since none of this is (yet) used in CI,
this is very low risk.

The only infinitessimally-risky change is using a dedicated
subdirectory of $WORKDIR (instead of $WORKDIR itself) as
the podman root. This fixes a dumb oversight on my part:
the workdir has grown to be used for much more than just
podman root; this change removes clutter and makes it
easier for humans to debug in cases of problems.

Signed-off-by: Ed Santiago <[email protected]>
@edsantiago edsantiago changed the title [WIP] apiv2 tests: add helpers to start/stop a local registry apiv2 tests: add helpers to start/stop a local registry Mar 9, 2021
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2021
@troyready
Copy link
Contributor

I'm having trouble running this (in the context of testing another PR #9589 ). Seems like it doesn't find any tests:

$ ./test/apiv2/test-apiv2 auth
Generating a RSA private key
.........................................++++
.........................................++++
writing new private key to '/tmp/test-apiv2.tmp.XUkD2D/registry/auth/domain.key'
-----
1..0

(trying it on debian testing aka bullseye, for context)

It wasn't immediately apparent to me what's causing it. If it could be some local system issue, happy to look into it after this gets merged but wanted to mention it now (while it's fresh on your mind) in case you might know offhand what would cause it.

@edsantiago
Copy link
Member Author

@troyready hand-edit test/apiv2/60-auth.at, remove the if and fi.

The confusion is my fault: I was happy with this PR, so I force-pushed it with the disabling conditional, with the goal of getting this merged soon so you can rebase on it.

@edsantiago
Copy link
Member Author

Failure is a flake, a new one: timeout in podman build. No possible way that it's my fault, so I've restarted.

@edsantiago
Copy link
Member Author

@containers/podman-maintainers PTAL. This is currently almost-entirely a NOP: it adds functionality intended for testing #9589 and potentially other auth-related endpoints. There's one minor functional change, for maintainability, but I've triple-convinced myself that it's safe.

CI is flaking, but it's my bedtime and I'm not hanging out to wait for it.

@TomSweeneyRedHat
Copy link
Member

As usual, really nice work @edsantiago
LGTM
and happy green test buttons.

@rhatdan
Copy link
Member

rhatdan commented Mar 10, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2021
@openshift-merge-robot openshift-merge-robot merged commit d98b55f into containers:master Mar 10, 2021
@edsantiago edsantiago deleted the apiv2_registry branch March 10, 2021 14:49
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants