Skip to content
This repository was archived by the owner on Oct 10, 2020. It is now read-only.

Generate RPM and support arbitrary files copied to the host#767

Closed
giuseppe wants to merge 18 commits intoprojectatomic:masterfrom
giuseppe:generate-rpm
Closed

Generate RPM and support arbitrary files copied to the host#767
giuseppe wants to merge 18 commits intoprojectatomic:masterfrom
giuseppe:generate-rpm

Conversation

@giuseppe
Copy link
Copy Markdown
Collaborator

@giuseppe giuseppe commented Nov 23, 2016

this is a prototype to add "pre installed" system containers. The idea is to have another deployment tree under /usr/lib/containers/atomic that is not handled by "atomic install/uninstall/update", where '/var/lib/containers/atomic' is fully handled by atomic. The systemd files are also copied under '/usr/lib'.

The second patch adds the possibility to install a system container into a rpm. The ---generate-rpm (three dashes) option is hidden for now. The spec file is auto generated, and stuff like Requires, Provides, Conflicts can be specified through labels. So in the Dockerfile something like: LABEL Provides="etcd", will be translated directly to the specfile.

Example:

# atomic --debug install --system ---generate-rpm gscrivano/etcd  
# rpm -i x86_64/atomic-container-etcd-1.0-1.0.x86_64.rpm

for root, _, files in os.walk(os.path.join(destdir, "etc")):
rel_path = os.path.relpath(root, destdir)
for f in files:
spec += "%config \"%s\"\n" % os.path.join("/", rel_path, f)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm confused, this line looks like you want to install container's /etc to host's /etc.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

here I am mapping from the temporary directory (where the container was checked out) to the absolute path `/etc' in the host (where the .rpm will install these files)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh sorry, I thought that the destdir variable points to the container's root.

system_xor_user.add_argument("--system", dest="system",
action='store_true', default=False,
help=_('install a system container'))
installp.add_argument("---generate-rpm", dest="generate_rpm",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer that the user not need to specify this. This should happen automatically if the image supports generation of rpms for install.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Users will forget to do this, and we want to eventually make this the default for all of our system containers.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

the --generate-rpm doesn't probably belong here, I just didn't know where to add it.

I think we will need a way to just generate the .rpm, so that it can be used as any regular .rpm, you can even install it without using atomic at all.

Installing the .rpm could be the default here, but we will need a way to specify also to install the container in the usual way using /var/lib instead of '/usr/lib' if needed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right I would say that is the fallback.
I believe the user puts the data in as templates rather then in labels. I am thinking a rpm.yaml file which specifies information for the spec file, but allows atomic CLI to do substitutions like the name of the container so the spec file ends up with foobar_etcd, and we could prefix all executables or paths with the container names.


values["DESTDIR"] = destination
if prefix:
values["DESTDIR"] = os.path.join("/", os.path.relpath(destination, prefix))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

values["DESTDIR"] = (os.path.join("/", os.path.relpath(destination), prefix)) if prefix else destination)

if os.path.exists(sym):
os.unlink(sym)
os.symlink(destination, sym)
if not prefix:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we just

if prefix:
     return

"--define", "_rpmdir %s" % cwd,
"--build-in-place",
"--buildroot=%s" % rpm_content]
util.write_out(" ".join(cmd))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if self.debug:

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Nov 23, 2016

I think there should be a rpm specification template in the container image that triggers the rpm build for install. This should be simpler then the rpm.spec, Perhaps just the files section as well as description, license, summary and potentially some other fields.

Should we default post install of rpm package to

atomic uninstall CONTAINER

@giuseppe
Copy link
Copy Markdown
Collaborator Author

We can add a spec template, as we do with the other configuration files (and offer a substitution for the list of the installed files, it would be very tedious to list them otherwise). I thought of using labels so we will be able to query resolves/provides/conflicts remotely without fetching the image first.

I have not added a preun script yet. I think it should just be to stop the systemd service. I think "atomic uninstall" should not interfere with rpm to handle/delete the files.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Nov 23, 2016

Right I guess we could get a circular dependency here.
I guess atomic uninstall should execute

rpm -e PACKAGE.

@giuseppe
Copy link
Copy Markdown
Collaborator Author

@rhatdan I have addressed your comments for the code.

I have pushed a bunch of other patches to cleanly support copied files to the host.

I have included the idea of @jfilak to copy files from /exports/hostfs to the host. These files are tracked by the rpm when generating one, otherwise they are managed by atomic and removed on "atomic uninstall".

The files copied to the host can be also template files, the same substitution rules that are used for the runc configuration file and the systemd service file apply here.

It is possible to specify that a system container doesn't install any service, so that can be used just as a way to copy files to the host, or to generate an rpm without the overhead of the service+rootfs files.

@giuseppe giuseppe force-pushed the generate-rpm branch 3 times, most recently from 5425cdd to 198f4f5 Compare November 28, 2016 14:36
@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Nov 30, 2016

@TomasTomecek PTAL

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Nov 30, 2016

@baude PTAL

@jfilak
Copy link
Copy Markdown

jfilak commented Nov 30, 2016

I like this pull request and I told @giuseppe that he has done a great job in private chat. However, there is one thing that concerns me a bit. I think it's a pity that the whole system container thing is bundled in 'atomic' utility. I develop my containers on Fedora and I would appreciate a tool capable of installing a test container on my development machine.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Nov 30, 2016

atomic command is available on Fedora as well as atomic host. It is a separate package.

@cgwalters
Copy link
Copy Markdown
Member

Atomic Host is part of Fedora. It is Fedora as much as a system managed via yum.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Nov 30, 2016

Right atomic CLI is available on Fedora Server, Fedora Workstation, Fedora Cloud/Atomic

@jfilak
Copy link
Copy Markdown

jfilak commented Dec 1, 2016

So it is possible to develop a DNF plugin installing system containers through 'atomic' utility. It would be cool to run:

dnf container-install gscrivano/etcd

if installed_files:
for i in installed_files:
spec = spec + "%s\n" % i

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it make sense to add changelog? So users know that the rpm is generated and installed by atomic.

@TomasTomecek
Copy link
Copy Markdown
Contributor

Got this traceback with image you provided to me:

$ sudo ./atomic --debug install --system ---generate-rpm --display img
Extracting to /tmp/tmprTqPaB/rpmroot/usr/lib/containers/atomic/img
./atomic --debug install --system ---generate-rpm --display img: too few arguments
Try './atomic --debug install --system ---generate-rpm --display img --help' for more information.
Traceback (most recent call last):
  File "./atomic", line 187, in <module>
    sys.exit(_func())
  File "/home/tt/g/atomic/Atomic/install.py", line 90, in install
    return self.syscontainers.install(self.image, self.name)
  File "/home/tt/g/atomic/Atomic/syscontainers.py", line 169, in install
    repo = self._get_ostree_repo()
  File "/home/tt/g/atomic/Atomic/syscontainers.py", line 1507, in generate_rpm
    self._checkout(repo, name, image, 0, False, destination=rootfs, prefix=rpm_content)
  File "/home/tt/g/atomic/Atomic/syscontainers.py", line 276, in _checkout
    return self._do_checkout(repo, name, img, upgrade, values, destination, unitfileout, tmpfilesout, extract_only, remote, prefix, installed_files
=installed_files)
  File "/home/tt/g/atomic/Atomic/syscontainers.py", line 363, in _do_checkout
    was_service_active = self._is_service_active(name)
  File "/home/tt/g/atomic/Atomic/syscontainers.py", line 965, in _is_service_active
    return self._systemctl_command("is-active", name, quiet=True).replace("\n", "") == "active"
AttributeError: 'NoneType' object has no attribute 'replace'

@giuseppe
Copy link
Copy Markdown
Collaborator Author

giuseppe commented Dec 1, 2016

@TomasTomecek --display is not really supported when generating an rpm as we need to read information from the files that are installed/checked out from the OSTree repository. I have added a fixup patch though to not cause the traceback when --display is used.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Dec 1, 2016

I would like to setup a BlueJeans tomorrow to help design this tool to make sure everyone is on the same page. We need this to work for use cases outside of --system containers as well.

@TomasTomecek
Copy link
Copy Markdown
Contributor

We need this to work for use cases outside of --system containers as well.

This would be awesome. Right now I'm in a process of writing down use cases which could benefit greatly from this feature. Please, invite me to the discussion.

@giuseppe
Copy link
Copy Markdown
Collaborator Author

giuseppe commented Dec 1, 2016

yes, this PR makes possible to install arbitrary files on the host and track them with a rpm without requiring a running or even installed container/systemd service.

Every file under /exports/hostfs will be copied to the host file system. If noContainerService is used in the manifest.json file then atomic won't try to install the systemd service and the runc container.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Dec 1, 2016

@giuseppe I scheduled the meeting tomorrow at 8:30 AM EST. I would like you to take the lead to describe what you are building and display different parts. Could you throw together a Google Doc in the mean time to help facilitate the discussion.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Dec 1, 2016

BTW Add anyone you would like to the meeting.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Dec 1, 2016

@TomasTomecek If you could write up your Use Cases, that would be helpful also.

@rh-atomic-bot
Copy link
Copy Markdown

☔ The latest upstream changes (presumably e405c6a) made this pull request unmergeable. Please resolve the merge conflicts.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
we have another mechanism to do this now.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
allow the $ORIGIN: prefix only when the branch is specified, otherwise it
is not decidable if $ORIGIN is the source to pull the image from or the
name of the image itself.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
when using system-package add to the default generated rpm only the
files that are exported to the host not every file that is part of the
container.  We will still be able to take advantage of the OSTree
storage.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This works only for not preinstalled containers (the ones under /usr)
that are handled separately.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Mar 22, 2017

@rh-atomic-bot r+ 4321643

@rh-atomic-bot
Copy link
Copy Markdown

⌛ Testing commit 4321643 with merge 6e1fe7a...

rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
Add a hidden (for now) option ---generate-rpm to "install --system" to
generate an rpm file instead of installing the container.  The rpm can
be installed on the system and the container won't be managed by
atomic, update and uninstall are disabled.

Differently from containers handled by atomic that installs the
container under /var/lib/containers/atomic, the RPM version installs
its files under /usr/lib/containers and can be integrated in an atomic
image as a normal rpm.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
Copy the /exports/hostfs/ to the host file system.

These files are tracked into the "info" file for a deployment.

Change update/rollback/uninstall to honor the new configuration.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
When generating an rpm, ha the files copied to the file system in the
rpm itself.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
It can be specified in the manifest.json file as:

    "installedFilesTemplate" : [
        "/usr/local/bin/hello-template.sh"
    ]

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
with the rpm generation, it can be useful to copy files to the host
without the overhead of maintaining a dummy service.

Containers that do not use a service, can specify it in the
manifest file as:

    "noContainerService": true

it also implies that the rootfs is deleted once all the files are
copied to the system.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
allow to rename a copied file to host using the variable substitution.

In the manifest.json file it is permitted to specify the destination
file name of a file copied to the host as:

    "renameFiles" : {
        "/usr/local/bin/hello-template.sh" : "/usr/local/bin/hello-template-$NAME.sh"
    },

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
Drop the hidden option ---generate-rpm in favor of --system-package.

The new option controls how a system container is installed to the host:

--generate-rpm=build build the rpm file without installing it.
--generate-rpm=yes build the rpm and install it, the rpm is deleted.
--generate-rpm=no do not attempt to build and install an rpm file.
--generate-rpm=auto install the rpm only if a .spec file is defined in
  the image.  This is the default.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
If an rpm file is already present the the container image. just use
it.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
it stores the image id used to install the container.  Use it in the
generated spec file to refer to the image ID.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
it contains the name of the image used to install the container

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
we have another mechanism to do this now.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
allow the $ORIGIN: prefix only when the branch is specified, otherwise it
is not decidable if $ORIGIN is the source to pull the image from or the
name of the image itself.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
when using system-package add to the default generated rpm only the
files that are exported to the host not every file that is part of the
container.  We will still be able to take advantage of the OSTree
storage.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Mar 22, 2017
This works only for not preinstalled containers (the ones under /usr)
that are handled separately.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #767
Approved by: rhatdan
@rh-atomic-bot
Copy link
Copy Markdown

☀️ Test successful - status-redhatci
Approved by: rhatdan
Pushing 6e1fe7a to master...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants