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

System Test - Quadlet kube oneshot #20363

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

ygalblum
Copy link
Contributor

Does this PR introduce a user-facing change?

No

None

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 15, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ygalblum

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 15, 2023
@ygalblum
Copy link
Contributor Author

@vrothberg this is the missing test from #20337

@ygalblum ygalblum force-pushed the quadlet-kube-oneshot-test branch from cd420e9 to 970b1f3 Compare October 15, 2023 10:06
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@edsantiago PTAL


run_podman container inspect --format "{{(index .Mounts 0).Type}}" $pod_name-$container_name
assert "$output" = "volume" \
"quadlet - kube oneshot: container should be attached to a volume of type volume"
Copy link
Member

Choose a reason for hiding this comment

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

Tests LGTM. For future reference (not a blocker), you don't need so much duplication duplication in the test descriptions. A simple "volume .Type" would be enough. Likewise above in line 1230, below 1241.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you meant here. The reason for the verbosity is to make the error line unique and explanatory. What do you suggest instead?

Copy link
Member

Choose a reason for hiding this comment

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

Just "volume .Type" would be fine. On failure, both Expected and Actual are shown, so there's no need to include Expected in the description. Not a big deal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"quadlet - kube oneshot: container should be attached to the volume named $volume_name"

# Shutdown the service and remove the volume
service_cleanup $pod_service inactive
Copy link
Member

Choose a reason for hiding this comment

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

If you need to repush, I would appreciate you adding

    run_podman rmi $(pause_image)

to avoid nasty red noise in logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ygalblum ygalblum force-pushed the quadlet-kube-oneshot-test branch from 970b1f3 to 6f7a42c Compare October 17, 2023 06:05
@ygalblum ygalblum force-pushed the quadlet-kube-oneshot-test branch from 6f7a42c to 6ba67ba Compare October 17, 2023 08:15
@edsantiago
Copy link
Member

/lgtm

thank you

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2023
@openshift-ci openshift-ci bot merged commit 5853e2b into containers:main Oct 17, 2023
98 checks passed
@ygalblum ygalblum deleted the quadlet-kube-oneshot-test branch October 17, 2023 11:33
@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 Jan 16, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 16, 2024
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. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants