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

Document .build for Image .container option #24926

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Jan 2, 2025

$NAME.build is supported in .container to automatically build the local image. This needs to be documented.

Also fix up other special cases to look the same in the man page.

Fixes: https://www.reddit.com/r/podman/comments/1hmhhhi/quadlet_build_units/?utm_source=share&utm_medium=web3x&utm_name=web3xcss&utm_term=1&utm_content=share_button

Does this PR introduce a user-facing change?

Man page now explains how to use .build from .container quadlets.

@openshift-ci openshift-ci bot added release-note approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 2, 2025
@rhatdan
Copy link
Member Author

rhatdan commented Jan 2, 2025

@ygalblum PTAL

Do you know if you can use .build from .Mount options or anywhere else where .image files can be used?

@ygalblum
Copy link
Contributor

ygalblum commented Jan 2, 2025

Do you know if you can use .build from .Mount options or anywhere else where .image files can be used?

Currently, no. The .build extension is currently resolved only for the Image key in .container or .volume files. Having said that, I think it can be added for the Mount key in the method handleStorageSource when checkImage is true

Note that the corresponding `.image` file must exist.
Special Cases:

* If the `name` of the image ends with `.image`, Quadlet will use the image pulled by the corresponding `.image` file, and the generated systemd service contains a dependency on the `$name-image.service`. Note that the corresponding `.image` file must exist.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since Quadlet now allows setting the name of the generated service, it should be something like:

"a dependency on the $name-image.service (or the service name set in the .image file)"

The same applies for the .build file and other places in this change

Copy link
Contributor

@ygalblum ygalblum left a comment

Choose a reason for hiding this comment

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

Sorry for noticing it only on my second review. Following the change that allowed setting the service name for unit, dependencies must be fulfilled also for .volume and .network files

created by using a `$name.volume` Quadlet file.
Special case:

* If `SOURCE-VOLUME` ends with `.volume`, a Podman named volume called `systemd-$name` is used as the source, and the generated systemd service contains a dependency on the `$name-volume.service`. Such a volume can be automatically be lazily created by using a `$name.volume` Quadlet file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Such a volume can be automatically be lazily created

I know this exists in the original doc, but it's incorrect. The .volume file must exist

Comment on lines 1035 to 1037
* If the `name` of the network ends with `.network`, Quadlet will look for the corresponding `.network` Quadlet unit. If found, Quadlet will use the name of the Network set in the Unit, otherwise, `systemd-$name` is used.

The generated systemd service contains a dependency on the service unit generated for that `.network` unit, or on `$name-network.service` if the `.network` unit is not found.
Copy link
Contributor

Choose a reason for hiding this comment

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

With the current implementation the .network file must exist

Comment on lines 1135 to 1139
* If `SOURCE-VOLUME` ends with `.volume`, Quadlet will look for the corresponding `.volume` Quadlet unit. If found, Quadlet will use the name of the Volume set in the Unit, otherwise, `systemd-$name` is used.

The generated systemd service contains a dependency on the service unit generated for that `.volume` unit,
or on `$name-volume.service` if the `.volume` unit is not found
or on `$name-volume.service` if the `.volume` unit is not found.
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, the .volume file must exist. Quadlet will use the name of the volume generated for the .volume unit

created by using a `$name.network` Quadlet file.
Special case:

* If the `name` of the network ends with `.network`, a Podman network called `systemd-$name` is used, and the generated systemd service contains a dependency on the `$name-network.service`. Such a network can be automatically created by using a `$name.network` Quadlet file.n
Copy link
Contributor

Choose a reason for hiding this comment

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

Such a network can be automatically created

The .network file must exist

unit is not found.
Special case:

* If the `name` of the network ends with `.network`, Quadlet will look for the corresponding `.network` Quadlet unit. If found, Quadlet will use the name of the Network set in the Unit, otherwise, `systemd-$name` is used. The generated systemd service contains a dependency on the service unit generated for that `.network` unit, or on `$name-network.service` if the `.network` unit is not found.
Copy link
Contributor

Choose a reason for hiding this comment

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

The .network file must exist

is not found
Special case:

* If `SOURCE-VOLUME` ends with `.volume`, Quadlet will look for the corresponding `.volume` Quadlet unit. If found, Quadlet will use the name of the Volume set in the Unit, otherwise, `systemd-$name` is used. The generated systemd service contains a dependency on the service unit generated for that `.volume` unit, or on `$name-volume.service` if the `.volume` unit is not found
Copy link
Contributor

Choose a reason for hiding this comment

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

The .volume file must exist

$NAME.build is supported in .container to automatically build the
local image. This needs to be documented.

Also fix up other special cases to look the same in the man page.

Fixes: https://www.reddit.com/r/podman/comments/1hmhhhi/quadlet_build_units/?utm_source=share&utm_medium=web3x&utm_name=web3xcss&utm_term=1&utm_content=share_button

Signed-off-by: Daniel J Walsh <[email protected]>
Copy link
Contributor

@ygalblum ygalblum left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 8, 2025
Copy link
Contributor

openshift-ci bot commented Jan 8, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, rhatdan, 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:
  • OWNERS [Luap99,rhatdan,ygalblum]

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

@openshift-merge-bot openshift-merge-bot bot merged commit 0cf2417 into containers:main Jan 8, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants