https://bugzilla.redhat.com/show_bug.cgi?id=1628914
Tomas Tomecek <ttomecek(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |MODIFIED
Flags|fedora-review- |fedora-review+
--- Comment #3 from Tomas Tomecek <ttomecek(a)redhat.com> ---
(In reply to Debarshi Ray from comment #2)
You mean that the docker:// prefix should be removed? It didn't
strike me as
a problem because it builds locally with 'buildah bud --tag
fedora-toolbox:28 .', but I can definitely remove it.
Yes, the `docker://` prefix.
I used docker to build it -- Fedora OSBS doesn't utilize buildah (yet?).
How were you building it?
> There are also some labels missing:
>
> * maintainer
The guidelines had a typo, which broke the example, and I wasn't sure if we
really wanted somebody's email as a point-of-contact since we already have
com.redhat.component.
Oh, I haven't noticed there is a typo in guidelines.
Feel free to open an issue on our tracker if you think that we should remove
the requirement for the maintainer label:
https://pagure.io/ContainerSIG/container-sig/issues
I skipped it because the image is primarily meant to be used with
the
fedora-toolbox command. I didn't want to encourage people to try it in some
other ad-hoc way. I can add a sample 'podman ...' command if you prefer it.
So please state it then in the usage label: 'This container image is meant to
be used with fedora-toolbox command.'
(Isn't 'atomic run' meant to be deprecated?)
I don't know. The containers team seem to be porting atomic features to podman
recently, e.g.
https://github.com/containers/libpod/pull/1463
> Not blocking this review:
>
> * doing update of all packages might break the application, since container
> images are tested as a unit: changing the unit might produce unknown results
Umm... what do you mean by application?
The primary purpose of this image is to be used with
https://github.com/debarshiray/fedora-toolbox to setup toolbox containers on
Fedora (see the README.md). Setting up a toolbox requires the passwd and
shadow-utils packages. The former is installed by the Dockerfile, and the
latter comes with the base image. So unless they disappear or suffer a
catastrophic breakage, things should mostly work.
On the other hand, not updating the packages on the server would mean that
the user would have to perform the same update on their own machine when
setting up the toolbox, which seems like a waste of time and bandwidth.
As I said: the container images are meant to be tested and provided as a unit.
When you perform an update inside, you change that unit and make it really hard
for the vendor to provide support for such image. If you want the newest
packages, you should wait for an updated base image (this is the best practices
which I can't find any documentation for).
This was really just a heads-up, feel free to leave it as it is: in the end,
it's your image, not mine :)
Also, images are all about the waste of time and bandwidth -- they just make a
very good packaging and distribution format and runtime which we all love at
the cost of bandwidth, time and bundling mess.
> * that mini-shell script is way too cryptic, why not just doing `cat
> /extra-packages | xargs dnf install -y`
Sadly, xargs comes from findutils and that's not part of the base fedora
image. That means adding a dnf command hard-coded to install findutils,
which (a) slows the build due to one more dnf process getting setup and run
(b) adds to the list of exceptions that's not covered by the extra-packages
file.
If you think the while loop is too cryptic, then I am happy to remove the
extra-packages indirection and list all the packages in the Dockerfile
itself.
Oh, I didn't realize that, really good point. Feel free to leave it as it is.
Rishi, I might be off the Internet for several weeks. I don't want you to be
blocked on me, hence granting review+. Please, just update the `FROM` line and
usage label and we should be good.
--
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug
https://bugzilla.redhat.com/token.cgi?t=0Hg9bx6W1j&a=cc_unsubscribe