DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v1] dts: fix runner target in the Dockerfile
@ 2024-09-11 15:50 jspewock
  2024-09-11 15:52 ` Luca Vizzarro
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: jspewock @ 2024-09-11 15:50 UTC (permalink / raw)
  To: paul.szczepanek, npratte, alex.chapman, Honnappa.Nagarahalli,
	juraj.linkes, Luca.Vizzarro, probb, thomas, yoan.picchi,
	wathsala.vithanage
  Cc: dev, Jeremy Spewock

From: Jeremy Spewock <jspewock@iol.unh.edu>

Currently the runner target in the Dockerfile attempts to run the
`poetry install` command when building the image, but this fails due to
poetry not being found in the container. Poetry is installed in a
previous step with pipx, but doing so adds the binary to use poetry to
~/.local/bin which isn't present in the PATH variable in the container
image. The command `pipx ensurepath` fixes this issue in most cases, but
it requires a restart of the shell in order for the changes to take
place which is not something that is done in the runner target. To
solve this problem this patch manually adds ~/.local/bin to PATH in the
runner target.

Additionally, the command for installing poetry in the runner target
uses the depreciated flag --no-dev which is removed in this patch and
replaced with the new method of doing the same thing alongside the
--no-root flag from the DTS documentation.

Fixes: 19082c1fac43 ("dts: add Dockerfile")
Cc: juraj.linkes@pantheon.tech

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/Dockerfile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/dts/Dockerfile b/dts/Dockerfile
index a81e46c41a..66b3cfd97f 100644
--- a/dts/Dockerfile
+++ b/dts/Dockerfile
@@ -24,7 +24,10 @@ FROM base AS runner
 # It bakes DTS into the image during the build.
 
 COPY . /dpdk/dts
-RUN poetry install --no-dev
+# Adds ~/.local/bin to PATH so that packages installed with pipx are callable. `pipx ensurepath`
+# fixes this issue, but requires the shell to be re-opened which isn't an option for this target.
+ENV PATH="$PATH:/root/.local/bin"
+RUN poetry install --only main --no-root
 
 CMD ["poetry", "run", "python", "main.py"]
 
-- 
2.46.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] dts: fix runner target in the Dockerfile
  2024-09-11 15:50 [PATCH v1] dts: fix runner target in the Dockerfile jspewock
@ 2024-09-11 15:52 ` Luca Vizzarro
  2024-09-11 17:26   ` Patrick Robb
  2024-09-16 10:16 ` Juraj Linkeš
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Luca Vizzarro @ 2024-09-11 15:52 UTC (permalink / raw)
  To: jspewock, paul.szczepanek, npratte, alex.chapman,
	Honnappa.Nagarahalli, juraj.linkes, probb, thomas, yoan.picchi,
	wathsala.vithanage
  Cc: dev

Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] dts: fix runner target in the Dockerfile
  2024-09-11 15:52 ` Luca Vizzarro
@ 2024-09-11 17:26   ` Patrick Robb
  0 siblings, 0 replies; 18+ messages in thread
From: Patrick Robb @ 2024-09-11 17:26 UTC (permalink / raw)
  To: Luca Vizzarro
  Cc: jspewock, paul.szczepanek, npratte, alex.chapman,
	Honnappa.Nagarahalli, juraj.linkes, thomas, yoan.picchi,
	wathsala.vithanage, dev

Reviewed-by: Patrick Robb <probb@iol.unh.edu>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] dts: fix runner target in the Dockerfile
  2024-09-11 15:50 [PATCH v1] dts: fix runner target in the Dockerfile jspewock
  2024-09-11 15:52 ` Luca Vizzarro
@ 2024-09-16 10:16 ` Juraj Linkeš
  2024-09-16 17:28   ` Jeremy Spewock
  2024-09-16 18:14 ` [PATCH v2] " jspewock
  2024-09-19 14:24 ` [PATCH v3] " jspewock
  3 siblings, 1 reply; 18+ messages in thread
From: Juraj Linkeš @ 2024-09-16 10:16 UTC (permalink / raw)
  To: jspewock, paul.szczepanek, npratte, alex.chapman,
	Honnappa.Nagarahalli, Luca.Vizzarro, probb, thomas, yoan.picchi,
	wathsala.vithanage
  Cc: dev



On 11. 9. 2024 17:50, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> Currently the runner target in the Dockerfile attempts to run the
> `poetry install` command when building the image, but this fails due to
> poetry not being found in the container. Poetry is installed in a
> previous step with pipx, but doing so adds the binary to use poetry to
> ~/.local/bin which isn't present in the PATH variable in the container
> image. The command `pipx ensurepath` fixes this issue in most cases, but
> it requires a restart of the shell in order for the changes to take
> place which is not something that is done in the runner target. To
> solve this problem this patch manually adds ~/.local/bin to PATH in the
> runner target.
> 
> Additionally, the command for installing poetry in the runner target
> uses the depreciated flag --no-dev which is removed in this patch and
> replaced with the new method of doing the same thing alongside the
> --no-root flag from the DTS documentation.
> 
> Fixes: 19082c1fac43 ("dts: add Dockerfile")
> Cc: juraj.linkes@pantheon.tech
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>   dts/Dockerfile | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/dts/Dockerfile b/dts/Dockerfile
> index a81e46c41a..66b3cfd97f 100644
> --- a/dts/Dockerfile
> +++ b/dts/Dockerfile
> @@ -24,7 +24,10 @@ FROM base AS runner
>   # It bakes DTS into the image during the build.
>   
>   COPY . /dpdk/dts
> -RUN poetry install --no-dev
> +# Adds ~/.local/bin to PATH so that packages installed with pipx are callable. `pipx ensurepath`
> +# fixes this issue, but requires the shell to be re-opened which isn't an option for this target.
> +ENV PATH="$PATH:/root/.local/bin"
> +RUN poetry install --only main --no-root

There's a patch from Dean that removed --no-root. I suggest waiting for 
Dean to submit v2, then you rebase on top of that and I'll merge both of 
these as we seem to have the tags.

>   
>   CMD ["poetry", "run", "python", "main.py"]
>   


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] dts: fix runner target in the Dockerfile
  2024-09-16 10:16 ` Juraj Linkeš
@ 2024-09-16 17:28   ` Jeremy Spewock
  0 siblings, 0 replies; 18+ messages in thread
From: Jeremy Spewock @ 2024-09-16 17:28 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: paul.szczepanek, npratte, alex.chapman, Honnappa.Nagarahalli,
	Luca.Vizzarro, probb, thomas, yoan.picchi, wathsala.vithanage,
	dev

On Mon, Sep 16, 2024 at 6:16 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
>
>
> On 11. 9. 2024 17:50, jspewock@iol.unh.edu wrote:
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Currently the runner target in the Dockerfile attempts to run the
> > `poetry install` command when building the image, but this fails due to
> > poetry not being found in the container. Poetry is installed in a
> > previous step with pipx, but doing so adds the binary to use poetry to
> > ~/.local/bin which isn't present in the PATH variable in the container
> > image. The command `pipx ensurepath` fixes this issue in most cases, but
> > it requires a restart of the shell in order for the changes to take
> > place which is not something that is done in the runner target. To
> > solve this problem this patch manually adds ~/.local/bin to PATH in the
> > runner target.
> >
> > Additionally, the command for installing poetry in the runner target
> > uses the depreciated flag --no-dev which is removed in this patch and
> > replaced with the new method of doing the same thing alongside the
> > --no-root flag from the DTS documentation.
> >
> > Fixes: 19082c1fac43 ("dts: add Dockerfile")
> > Cc: juraj.linkes@pantheon.tech
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> >   dts/Dockerfile | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/dts/Dockerfile b/dts/Dockerfile
> > index a81e46c41a..66b3cfd97f 100644
> > --- a/dts/Dockerfile
> > +++ b/dts/Dockerfile
> > @@ -24,7 +24,10 @@ FROM base AS runner
> >   # It bakes DTS into the image during the build.
> >
> >   COPY . /dpdk/dts
> > -RUN poetry install --no-dev
> > +# Adds ~/.local/bin to PATH so that packages installed with pipx are callable. `pipx ensurepath`
> > +# fixes this issue, but requires the shell to be re-opened which isn't an option for this target.
> > +ENV PATH="$PATH:/root/.local/bin"
> > +RUN poetry install --only main --no-root
>
> There's a patch from Dean that removed --no-root. I suggest waiting for
> Dean to submit v2, then you rebase on top of that and I'll merge both of
> these as we seem to have the tags.

I didn't know about this change but this makes sense to me. There is
one other version that I'll submit here before Dean puts out his V2
since we had further discussion on slack about it potentially being
beneficial to add an ENTRYPOINT to the dockerfile, as well as fixing
the missing git package.

>
> >
> >   CMD ["poetry", "run", "python", "main.py"]
> >
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-11 15:50 [PATCH v1] dts: fix runner target in the Dockerfile jspewock
  2024-09-11 15:52 ` Luca Vizzarro
  2024-09-16 10:16 ` Juraj Linkeš
@ 2024-09-16 18:14 ` jspewock
  2024-09-17  9:50   ` Luca Vizzarro
                     ` (2 more replies)
  2024-09-19 14:24 ` [PATCH v3] " jspewock
  3 siblings, 3 replies; 18+ messages in thread
From: jspewock @ 2024-09-16 18:14 UTC (permalink / raw)
  To: npratte, probb, wathsala.vithanage, yoan.picchi, juraj.linkes,
	paul.szczepanek, alex.chapman, Luca.Vizzarro, thomas,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

From: Jeremy Spewock <jspewock@iol.unh.edu>

Currently the runner target in the Dockerfile attempts to run the
`poetry install` command when building the image, but this fails due to
poetry not being found in the container. Poetry is installed in a
previous step with pipx, but doing so adds the binary to use poetry to
~/.local/bin which isn't present in the PATH variable in the container
image. The command `pipx ensurepath` fixes this issue in most cases, but
it requires a restart of the shell in order for the changes to take
place which is not something that can be done in the runner target. To
solve this problem this patch manually adds ~/.local/bin to PATH in the
runner target.

Additionally, the command for installing poetry in the runner target
uses a depreciated flag, and the --revision parameter does not work with
the runner target. To address these problems the --no-dev flag is
removed in this patch and replaced with the new method of doing the same
thing alongside the --no-root flag from the DTS documentation and git is
added to the base target of the image. The CMD of the runner target is
also replaced with an ENTRYPOINT for ease of use.

Fixes: 19082c1fac43 ("dts: add Dockerfile")
Cc: juraj.linkes@pantheon.tech

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/Dockerfile | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/dts/Dockerfile b/dts/Dockerfile
index a81e46c41a..58642fdc67 100644
--- a/dts/Dockerfile
+++ b/dts/Dockerfile
@@ -13,8 +13,11 @@ RUN apt-get -y update && apt-get -y upgrade && \
         python3-pip \
         pipx \
         python3-cachecontrol \
+        git \
+        xz-utils \
         openssh-client && \
-    pipx install poetry>=1.8.2 && pipx ensurepath
+    pipx install poetry>=1.8.2 && pipx ensurepath && \
+    git config --global --add safe.directory /dpdk
 WORKDIR /dpdk/dts
 
 
@@ -24,9 +27,12 @@ FROM base AS runner
 # It bakes DTS into the image during the build.
 
 COPY . /dpdk/dts
-RUN poetry install --no-dev
+# Adds ~/.local/bin to PATH so that packages installed with pipx are callable. `pipx ensurepath`
+# fixes this issue, but requires the shell to be re-opened which isn't an option for this target.
+ENV PATH="$PATH:/root/.local/bin"
+RUN poetry install --only main --no-root
 
-CMD ["poetry", "run", "python", "main.py"]
+ENTRYPOINT ["poetry", "run", "python", "main.py"]
 
 FROM base AS dev
 
@@ -35,4 +41,4 @@ FROM base AS dev
 # the dependencies should be installed using Poetry.
 
 RUN apt-get -y install --no-install-recommends \
-        vim emacs git
+        vim emacs
-- 
2.46.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-16 18:14 ` [PATCH v2] " jspewock
@ 2024-09-17  9:50   ` Luca Vizzarro
  2024-09-17 15:22     ` Patrick Robb
  2024-09-18  7:57   ` Juraj Linkeš
  2024-09-19  9:36   ` Juraj Linkeš
  2 siblings, 1 reply; 18+ messages in thread
From: Luca Vizzarro @ 2024-09-17  9:50 UTC (permalink / raw)
  To: jspewock, npratte, probb, wathsala.vithanage, yoan.picchi,
	juraj.linkes, paul.szczepanek, alex.chapman, thomas,
	Honnappa.Nagarahalli
  Cc: dev

Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-17  9:50   ` Luca Vizzarro
@ 2024-09-17 15:22     ` Patrick Robb
  2024-09-18  7:44       ` Juraj Linkeš
  0 siblings, 1 reply; 18+ messages in thread
From: Patrick Robb @ 2024-09-17 15:22 UTC (permalink / raw)
  To: Luca Vizzarro
  Cc: jspewock, npratte, wathsala.vithanage, yoan.picchi, juraj.linkes,
	paul.szczepanek, alex.chapman, thomas, Honnappa.Nagarahalli, dev

Dean actually just came down with an illness. Since the changes in his
series are minimal (I see the only ones left are updating dts.rst and
the devcontainer json) I think that one of us at UNH should submit a
patch which is essentially a v2 of his series. It's probably not worth
blocking a "critical" bugfix for an extended period just to avoid
moving patches between people.

Juraj if you think this is okay I can try to do this tonight.

On Tue, Sep 17, 2024 at 5:50 AM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
>
> Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-17 15:22     ` Patrick Robb
@ 2024-09-18  7:44       ` Juraj Linkeš
  0 siblings, 0 replies; 18+ messages in thread
From: Juraj Linkeš @ 2024-09-18  7:44 UTC (permalink / raw)
  To: Patrick Robb, Luca Vizzarro
  Cc: jspewock, npratte, wathsala.vithanage, yoan.picchi,
	paul.szczepanek, alex.chapman, thomas, Honnappa.Nagarahalli, dev



On 17. 9. 2024 17:22, Patrick Robb wrote:
> Dean actually just came down with an illness. Since the changes in his
> series are minimal (I see the only ones left are updating dts.rst and
> the devcontainer json) I think that one of us at UNH should submit a
> patch which is essentially a v2 of his series. It's probably not worth
> blocking a "critical" bugfix for an extended period just to avoid
> moving patches between people.
> 
> Juraj if you think this is okay I can try to do this tonight.
> 

No problem. I think you should add your signoff if you do so.

We could also just wait for Dean to return - we merge this patch and 
then Dean can make the change on top of it. Both are fine.

> On Tue, Sep 17, 2024 at 5:50 AM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
>>
>> Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-16 18:14 ` [PATCH v2] " jspewock
  2024-09-17  9:50   ` Luca Vizzarro
@ 2024-09-18  7:57   ` Juraj Linkeš
  2024-09-18 14:16     ` Jeremy Spewock
  2024-09-19  9:36   ` Juraj Linkeš
  2 siblings, 1 reply; 18+ messages in thread
From: Juraj Linkeš @ 2024-09-18  7:57 UTC (permalink / raw)
  To: jspewock, npratte, probb, wathsala.vithanage, yoan.picchi,
	paul.szczepanek, alex.chapman, Luca.Vizzarro, thomas,
	Honnappa.Nagarahalli
  Cc: dev


> diff --git a/dts/Dockerfile b/dts/Dockerfile

> @@ -24,9 +27,12 @@ FROM base AS runner

> +# Adds ~/.local/bin to PATH so that packages installed with pipx are callable. `pipx ensurepath`
> +# fixes this issue, but requires the shell to be re-opened which isn't an option for this target.

Let's explain this a bit more, I don't really know why this isn't an option.

> +ENV PATH="$PATH:/root/.local/bin"
> +RUN poetry install --only main --no-root
>   
> -CMD ["poetry", "run", "python", "main.py"]
> +ENTRYPOINT ["poetry", "run", "python", "main.py"]
>   

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-18  7:57   ` Juraj Linkeš
@ 2024-09-18 14:16     ` Jeremy Spewock
  2024-09-19  7:09       ` Juraj Linkeš
  0 siblings, 1 reply; 18+ messages in thread
From: Jeremy Spewock @ 2024-09-18 14:16 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: npratte, probb, wathsala.vithanage, yoan.picchi, paul.szczepanek,
	alex.chapman, Luca.Vizzarro, thomas, Honnappa.Nagarahalli, dev

On Wed, Sep 18, 2024 at 3:57 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
>
> > diff --git a/dts/Dockerfile b/dts/Dockerfile
>
> > @@ -24,9 +27,12 @@ FROM base AS runner
>
> > +# Adds ~/.local/bin to PATH so that packages installed with pipx are callable. `pipx ensurepath`
> > +# fixes this issue, but requires the shell to be re-opened which isn't an option for this target.
>
> Let's explain this a bit more, I don't really know why this isn't an option.

The main reason it isn't an option is it is all happening in the same
`docker build` process and it seems like however Docker is deciding to
create the layers isn't refreshing the terminal. I don't think there
is a way we could make it do so, but I can swap the "isn't an option"
part of the comment for something more like "and the build process
does not refresh the terminal in the required way before creating the
next layer."

>
> > +ENV PATH="$PATH:/root/.local/bin"
> > +RUN poetry install --only main --no-root
> >
> > -CMD ["poetry", "run", "python", "main.py"]
> > +ENTRYPOINT ["poetry", "run", "python", "main.py"]
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-18 14:16     ` Jeremy Spewock
@ 2024-09-19  7:09       ` Juraj Linkeš
  2024-09-19 14:12         ` Jeremy Spewock
  0 siblings, 1 reply; 18+ messages in thread
From: Juraj Linkeš @ 2024-09-19  7:09 UTC (permalink / raw)
  To: Jeremy Spewock
  Cc: npratte, probb, wathsala.vithanage, yoan.picchi, paul.szczepanek,
	alex.chapman, Luca.Vizzarro, thomas, Honnappa.Nagarahalli, dev



On 18. 9. 2024 16:16, Jeremy Spewock wrote:
> On Wed, Sep 18, 2024 at 3:57 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>>
>>> diff --git a/dts/Dockerfile b/dts/Dockerfile
>>
>>> @@ -24,9 +27,12 @@ FROM base AS runner
>>
>>> +# Adds ~/.local/bin to PATH so that packages installed with pipx are callable. `pipx ensurepath`
>>> +# fixes this issue, but requires the shell to be re-opened which isn't an option for this target.
>>
>> Let's explain this a bit more, I don't really know why this isn't an option.
> 
> The main reason it isn't an option is it is all happening in the same
> `docker build` process and it seems like however Docker is deciding to
> create the layers isn't refreshing the terminal. I don't think there
> is a way we could make it do so, but I can swap the "isn't an option"
> part of the comment for something more like "and the build process
> does not refresh the terminal in the required way before creating the
> next layer."
> 

Yea I think that's because each layer is basically independent so env 
vars (likely set by `pipx ensurepath`) don't persist.

I understand it now. The were multiple confusing things (how does `pipx 
ensurepath` relate to ~/.local/bin, how does `pipx ensurepath` fix which 
issue, why is shell mentioned and what option is not an option and why 
for this target and not the other and possibly more). It basically 
raised more questions than answered. :-)

I would just say:
pipx installs packages in ~/.local/bin, which is not in PATH by default. 
The `pipx ensurepath` command (which sets the path) called in the 
previous step only works in that step as the docker build process 
doesn't preserve env variables between steps.

Or something like that.

>>
>>> +ENV PATH="$PATH:/root/.local/bin"
>>> +RUN poetry install --only main --no-root
>>>
>>> -CMD ["poetry", "run", "python", "main.py"]
>>> +ENTRYPOINT ["poetry", "run", "python", "main.py"]
>>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-16 18:14 ` [PATCH v2] " jspewock
  2024-09-17  9:50   ` Luca Vizzarro
  2024-09-18  7:57   ` Juraj Linkeš
@ 2024-09-19  9:36   ` Juraj Linkeš
  2024-09-19 11:28     ` Juraj Linkeš
  2 siblings, 1 reply; 18+ messages in thread
From: Juraj Linkeš @ 2024-09-19  9:36 UTC (permalink / raw)
  To: jspewock, npratte, probb, wathsala.vithanage, yoan.picchi,
	paul.szczepanek, alex.chapman, Luca.Vizzarro, thomas,
	Honnappa.Nagarahalli
  Cc: dev


> diff --git a/dts/Dockerfile b/dts/Dockerfile

> @@ -24,9 +27,12 @@ FROM base AS runner
>   # It bakes DTS into the image during the build.
>   
>   COPY . /dpdk/dts
> -RUN poetry install --no-dev
> +# Adds ~/.local/bin to PATH so that packages installed with pipx are callable. `pipx ensurepath`
> +# fixes this issue, but requires the shell to be re-opened which isn't an option for this target.
> +ENV PATH="$PATH:/root/.local/bin"
> +RUN poetry install --only main --no-root

The patch removing this has been merged to next-dts, so please rebase 
and remove this.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-19  9:36   ` Juraj Linkeš
@ 2024-09-19 11:28     ` Juraj Linkeš
  0 siblings, 0 replies; 18+ messages in thread
From: Juraj Linkeš @ 2024-09-19 11:28 UTC (permalink / raw)
  To: jspewock, npratte, probb, wathsala.vithanage, yoan.picchi,
	paul.szczepanek, alex.chapman, Luca.Vizzarro, thomas,
	Honnappa.Nagarahalli
  Cc: dev



On 19. 9. 2024 11:36, Juraj Linkeš wrote:
> 
>> diff --git a/dts/Dockerfile b/dts/Dockerfile
> 
>> @@ -24,9 +27,12 @@ FROM base AS runner
>>   # It bakes DTS into the image during the build.
>>   COPY . /dpdk/dts
>> -RUN poetry install --no-dev
>> +# Adds ~/.local/bin to PATH so that packages installed with pipx are 
>> callable. `pipx ensurepath`
>> +# fixes this issue, but requires the shell to be re-opened which 
>> isn't an option for this target.
>> +ENV PATH="$PATH:/root/.local/bin"
>> +RUN poetry install --only main --no-root
> 
> The patch removing this has been merged to next-dts, so please rebase 
> and remove this.
> 
> 

And by this I mean "--no-root". :-)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] dts: fix runner target in the Dockerfile
  2024-09-19  7:09       ` Juraj Linkeš
@ 2024-09-19 14:12         ` Jeremy Spewock
  0 siblings, 0 replies; 18+ messages in thread
From: Jeremy Spewock @ 2024-09-19 14:12 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: npratte, probb, wathsala.vithanage, yoan.picchi, paul.szczepanek,
	alex.chapman, Luca.Vizzarro, thomas, Honnappa.Nagarahalli, dev

On Thu, Sep 19, 2024 at 3:09 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
>
>
> On 18. 9. 2024 16:16, Jeremy Spewock wrote:
> > On Wed, Sep 18, 2024 at 3:57 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> >>
> >>
> >>> diff --git a/dts/Dockerfile b/dts/Dockerfile
> >>
> >>> @@ -24,9 +27,12 @@ FROM base AS runner
> >>
> >>> +# Adds ~/.local/bin to PATH so that packages installed with pipx are callable. `pipx ensurepath`
> >>> +# fixes this issue, but requires the shell to be re-opened which isn't an option for this target.
> >>
> >> Let's explain this a bit more, I don't really know why this isn't an option.
> >
> > The main reason it isn't an option is it is all happening in the same
> > `docker build` process and it seems like however Docker is deciding to
> > create the layers isn't refreshing the terminal. I don't think there
> > is a way we could make it do so, but I can swap the "isn't an option"
> > part of the comment for something more like "and the build process
> > does not refresh the terminal in the required way before creating the
> > next layer."
> >
>
> Yea I think that's because each layer is basically independent so env
> vars (likely set by `pipx ensurepath`) don't persist.
>
> I understand it now. The were multiple confusing things (how does `pipx
> ensurepath` relate to ~/.local/bin, how does `pipx ensurepath` fix which
> issue, why is shell mentioned and what option is not an option and why
> for this target and not the other and possibly more). It basically
> raised more questions than answered. :-)
>
> I would just say:
> pipx installs packages in ~/.local/bin, which is not in PATH by default.
> The `pipx ensurepath` command (which sets the path) called in the
> previous step only works in that step as the docker build process

What's interesting as well is the pipx ensurepath command does fix the
issue when you start the interactive dev target. So it doesn't persist
between layers, but it does in the final container.

> doesn't preserve env variables between steps.
>
> Or something like that.
>
> >>
> >>> +ENV PATH="$PATH:/root/.local/bin"
> >>> +RUN poetry install --only main --no-root
> >>>
> >>> -CMD ["poetry", "run", "python", "main.py"]
> >>> +ENTRYPOINT ["poetry", "run", "python", "main.py"]
> >>>
>

-- 



*Let's Connect!*

...  *October Webinars*

Ask Us Anything: IOL Services 
Open Q&A 
<https://unh.zoom.us/webinar/register/9017265932716/WN_OUo5S7iQRLmKKY7CsmwZhw#/registration>Your 
questions. Our answers. Let's get started.


Oct 3rd


Live Tour of INTACT® 
for IPv6 Testing and Validation 
<https://unh.zoom.us/webinar/register/7117231236474/WN_I2zfyi_2S2yEiXkxBRi8sA#/registration>
Open tour. Open Q&A. See why we think you'll love INTACT.

Oct 9th


How to 
Prep for Our NVMe® Plugfest #21 
<https://unh.zoom.us/webinar/register/4017266809553/WN_X1iA2SZ8QhmcGboF2DImNg#/registration>
Checklists. Conversation. Let's get ready to plugin! 
Oct 15th


... * 
Newsletter*

*
*
Get the IOL Connector 
<https://www.iol.unh.edu/news/email-newsletters> for our lastest news and 
event info.



.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3] dts: fix runner target in the Dockerfile
  2024-09-11 15:50 [PATCH v1] dts: fix runner target in the Dockerfile jspewock
                   ` (2 preceding siblings ...)
  2024-09-16 18:14 ` [PATCH v2] " jspewock
@ 2024-09-19 14:24 ` jspewock
  2024-09-19 14:34   ` Luca Vizzarro
  2024-09-19 14:35   ` Juraj Linkeš
  3 siblings, 2 replies; 18+ messages in thread
From: jspewock @ 2024-09-19 14:24 UTC (permalink / raw)
  To: probb, Luca.Vizzarro, thomas, yoan.picchi, Honnappa.Nagarahalli,
	npratte, alex.chapman, paul.szczepanek, juraj.linkes,
	wathsala.vithanage
  Cc: dev, Jeremy Spewock

From: Jeremy Spewock <jspewock@iol.unh.edu>

Currently the runner target in the Dockerfile attempts to run the
`poetry install` command when building the image, but this fails due to
poetry not being found in the container. Poetry is installed in a
previous step with pipx, but doing so adds the binary to use poetry to
~/.local/bin which isn't present in the PATH variable in the container
image. The command `pipx ensurepath` fixes this issue in most cases, but
it requires a restart of the shell in order for the changes to take
place which is not something that can be done in the runner target. To
solve this problem this patch manually adds ~/.local/bin to PATH in the
runner target.

Additionally, the command for installing poetry in the runner target
uses a depreciated flag, and the --revision parameter does not work with
the runner target. To address these problems the --no-dev flag is
removed in this patch and replaced with the new method of doing the same
thing and git is added to the base target of the image. The CMD of the
runner target is also replaced with an ENTRYPOINT for ease of use.

Fixes: 19082c1fac43 ("dts: add Dockerfile")
Cc: juraj.linkes@pantheon.tech

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/Dockerfile | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/dts/Dockerfile b/dts/Dockerfile
index a81e46c41a..cb23faf3d4 100644
--- a/dts/Dockerfile
+++ b/dts/Dockerfile
@@ -13,8 +13,11 @@ RUN apt-get -y update && apt-get -y upgrade && \
         python3-pip \
         pipx \
         python3-cachecontrol \
+        git \
+        xz-utils \
         openssh-client && \
-    pipx install poetry>=1.8.2 && pipx ensurepath
+    pipx install poetry>=1.8.2 && pipx ensurepath && \
+    git config --global --add safe.directory /dpdk
 WORKDIR /dpdk/dts
 
 
@@ -24,9 +27,14 @@ FROM base AS runner
 # It bakes DTS into the image during the build.
 
 COPY . /dpdk/dts
-RUN poetry install --no-dev
+# pipx installs packages in ~/.local/bin, which is not in PATH by default. The `pipx ensurepath`
+# command used in the previous step adds said directory to PATH, but the docker build process does
+# not preserve environment variables between steps. Therefore, ~/.local/bin must be manually added
+# into PATH in order to use the poetry command below.
+ENV PATH="$PATH:/root/.local/bin"
+RUN poetry install --only main
 
-CMD ["poetry", "run", "python", "main.py"]
+ENTRYPOINT ["poetry", "run", "python", "main.py"]
 
 FROM base AS dev
 
@@ -35,4 +43,4 @@ FROM base AS dev
 # the dependencies should be installed using Poetry.
 
 RUN apt-get -y install --no-install-recommends \
-        vim emacs git
+        vim emacs
-- 
2.46.0


-- 



*Let's Connect!*

...  *October Webinars*

Ask Us Anything: IOL Services 
Open Q&A 
<https://unh.zoom.us/webinar/register/9017265932716/WN_OUo5S7iQRLmKKY7CsmwZhw#/registration>Your 
questions. Our answers. Let's get started.


Oct 3rd


Live Tour of INTACT(R) 
for IPv6 Testing and Validation 
<https://unh.zoom.us/webinar/register/7117231236474/WN_I2zfyi_2S2yEiXkxBRi8sA#/registration>
Open tour. Open Q&A. See why we think you'll love INTACT.

Oct 9th


How to 
Prep for Our NVMe(R) Plugfest #21 
<https://unh.zoom.us/webinar/register/4017266809553/WN_X1iA2SZ8QhmcGboF2DImNg#/registration>
Checklists. Conversation. Let's get ready to plugin! 
Oct 15th


... * 
Newsletter*

*
*
Get the IOL Connector 
<https://www.iol.unh.edu/news/email-newsletters> for our latest news and 
event info.



.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] dts: fix runner target in the Dockerfile
  2024-09-19 14:24 ` [PATCH v3] " jspewock
@ 2024-09-19 14:34   ` Luca Vizzarro
  2024-09-19 14:35   ` Juraj Linkeš
  1 sibling, 0 replies; 18+ messages in thread
From: Luca Vizzarro @ 2024-09-19 14:34 UTC (permalink / raw)
  To: jspewock, probb, thomas, yoan.picchi, Honnappa.Nagarahalli,
	npratte, alex.chapman, paul.szczepanek, juraj.linkes,
	wathsala.vithanage
  Cc: dev

Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] dts: fix runner target in the Dockerfile
  2024-09-19 14:24 ` [PATCH v3] " jspewock
  2024-09-19 14:34   ` Luca Vizzarro
@ 2024-09-19 14:35   ` Juraj Linkeš
  1 sibling, 0 replies; 18+ messages in thread
From: Juraj Linkeš @ 2024-09-19 14:35 UTC (permalink / raw)
  To: jspewock, probb, Luca.Vizzarro, thomas, yoan.picchi,
	Honnappa.Nagarahalli, npratte, alex.chapman, paul.szczepanek,
	wathsala.vithanage
  Cc: dev



On 19. 9. 2024 16:24, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> Currently the runner target in the Dockerfile attempts to run the
> `poetry install` command when building the image, but this fails due to
> poetry not being found in the container. Poetry is installed in a
> previous step with pipx, but doing so adds the binary to use poetry to
> ~/.local/bin which isn't present in the PATH variable in the container
> image. The command `pipx ensurepath` fixes this issue in most cases, but
> it requires a restart of the shell in order for the changes to take
> place which is not something that can be done in the runner target. To
> solve this problem this patch manually adds ~/.local/bin to PATH in the
> runner target.
> 
> Additionally, the command for installing poetry in the runner target
> uses a depreciated flag, and the --revision parameter does not work with
> the runner target. To address these problems the --no-dev flag is
> removed in this patch and replaced with the new method of doing the same
> thing and git is added to the base target of the image. The CMD of the
> runner target is also replaced with an ENTRYPOINT for ease of use.
> 
> Fixes: 19082c1fac43 ("dts: add Dockerfile")
> Cc: juraj.linkes@pantheon.tech
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>

Applied to next-dts after adding review-by tags. Thanks.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-09-19 14:35 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-11 15:50 [PATCH v1] dts: fix runner target in the Dockerfile jspewock
2024-09-11 15:52 ` Luca Vizzarro
2024-09-11 17:26   ` Patrick Robb
2024-09-16 10:16 ` Juraj Linkeš
2024-09-16 17:28   ` Jeremy Spewock
2024-09-16 18:14 ` [PATCH v2] " jspewock
2024-09-17  9:50   ` Luca Vizzarro
2024-09-17 15:22     ` Patrick Robb
2024-09-18  7:44       ` Juraj Linkeš
2024-09-18  7:57   ` Juraj Linkeš
2024-09-18 14:16     ` Jeremy Spewock
2024-09-19  7:09       ` Juraj Linkeš
2024-09-19 14:12         ` Jeremy Spewock
2024-09-19  9:36   ` Juraj Linkeš
2024-09-19 11:28     ` Juraj Linkeš
2024-09-19 14:24 ` [PATCH v3] " jspewock
2024-09-19 14:34   ` Luca Vizzarro
2024-09-19 14:35   ` Juraj Linkeš

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).