[PATCH 1/1] forward SSTATE_MIRRORS environment variable

[Felix Moessbauer]

When using the sstate cache in a CI system, the cache is likely accessed
via the SSTATE_MIRRORS env var (in contrast to local builds where it is
accessed via SSTATE_DIR). By that, we also need to automatically forward
this variable.

Previously, users had to manually add this variable to the env section
of a kas file. This led to some confusion, as local accesses to the
cache worked, but not when running in a CI.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---
docs/command-line.rst | 1 +
kas/libkas.py | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/command-line.rst b/docs/command-line.rst
index 39ee2ff..25be620 100644
--- a/docs/command-line.rst
+++ b/docs/command-line.rst
@@ -66,6 +66,7 @@ Environment variables
+--------------------------+--------------------------------------------------+
| ``DL_DIR`` | Environment variables that are transferred to |
| ``SSTATE_DIR`` | the bitbake environment. |
+| ``SSTATE_MIRRORS`` | |
| ``TMPDIR`` | |
+--------------------------+--------------------------------------------------+
| ``http_proxy`` | These variables define the proxy configuration |
diff --git a/kas/libkas.py b/kas/libkas.py
index 613b38a..24d55ff 100644
--- a/kas/libkas.py
+++ b/kas/libkas.py
@@ -278,7 +278,7 @@ def get_build_environ(build_system):

conf_env = get_context().config.get_environment()

- env_vars = ['SSTATE_DIR', 'DL_DIR', 'TMPDIR']
+ env_vars = ['SSTATE_DIR', 'SSTATE_MIRRORS', 'DL_DIR', 'TMPDIR']
env_vars.extend(conf_env)

env.update(conf_env)
--
2.39.2

[Schmidt, Adriaan]

Acked-by: Adriaan Schmidt <adriaan...@siemens.com>


MOESSBAUER, Felix (T CED INW-CN) <felix.mo...@siemens.com>, Thursday, August 3, 2023 10:09 AM

[Jose Quaresma]

'Felix Moessbauer' via kas-devel <kas-...@googlegroups.com> escreveu no dia quinta, 3/08/2023 à(s) 09:09:

When using the sstate cache in a CI system, the cache is likely accessed
via the SSTATE_MIRRORS env var (in contrast to local builds where it is
accessed via SSTATE_DIR). By that, we also need to automatically forward
this variable.

I think the description can be rewritten as it is not quite true.
The cache is always stored on the SSTATE_DIR and is the first place where the sstate.bbclass looks for it to speed up tasks,

when the cache files not found on the SSTATE_DIR they will be searched on SSTATE_MIRRORS.
Another difference is that the SSTATE_MIRRORS is read only while the SSTATE_DIR was read-write.

Jose
 

--
You received this message because you are subscribed to the Google Groups "kas-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kas-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kas-devel/20230803080854.214555-1-felix.moessbauer%40siemens.com.

--

Best regards,

José Quaresma

[Felix Moessbauer]

In case cache artifacts are not found in SSTATE_DIR, the sstate logic
continues to search in the SSTATE_MIRRORS. By that, we also need

to automatically forward this variable.

Previously, users had to manually add this variable to the env
section of a kas file.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

Changes since v1:

- reworked commit message

Best regards,
Felix

[Jan Kiszka]

What about kas-container?

Jan

--
Siemens AG, Technology
Linux Expert Center

[MOESSBAUER, Felix]

This was not in scope here, as the content of the SSTATE_MIRRORS
variable can be very system specific (e.g. it can contain local file
paths which cannot easily be mapped to paths inside the container).

But I'll add a second commit that at least covers the case that only
remote targets are added to the mirrors.

Felix

> >
> > Jan
> >

[Felix Moessbauer]

Changes since v2:

- p2: forward SSTATE_MIRRORS in kas-container as well

Changes since v1:

- reworked commit message

Best regards,

Felix Moessbauer
Siemens AG

Felix Moessbauer (2):
forward SSTATE_MIRRORS environment variable
kas-container: forward SSTATE_MIRRORS env var

docs/command-line.rst | 1 +
kas-container | 8 ++++++++
kas/libkas.py | 2 +-
3 files changed, 10 insertions(+), 1 deletion(-)

--
2.39.2

[Felix Moessbauer]

In case cache artifacts are not found in SSTATE_DIR, the sstate logic
continues to search in the SSTATE_MIRRORS. By that, we also need
to automatically forward this variable.

Previously, users had to manually add this variable to the env
section of a kas file.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

--
2.39.2

[Felix Moessbauer]

This variable is forwarded 1:1 into the container. In case it contains
a local path (starting with file:///), we issue a warning that this
path manually needs to be made available inside the container.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

kas-container | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/kas-container b/kas-container
index 36784cb..626ed6d 100755
--- a/kas-container
+++ b/kas-container
@@ -504,6 +504,14 @@ if [ -n "${SSTATE_DIR}" ]; then
-v "$(readlink -fv "${SSTATE_DIR}")":/sstate:rw \
-e SSTATE_DIR=/sstate
fi
+if [ -n "${SSTATE_MIRRORS}" ]; then
+ if echo "${SSTATE_MIRRORS}" | grep -q "file:///"; then
+ echo "Warning: SSTATE_MIRRORS contains a local path." \
+ "Make sure to make this path available inside the container." \
+ >&2
+ fi
+ set -- "$@" -e "SSTATE_MIRRORS=${SSTATE_MIRRORS}"
+fi

if [ -n "${KAS_REPO_REF_DIR}" ]; then
if [ ! -d "${KAS_REPO_REF_DIR}" ]; then
--
2.39.2

[Jan Kiszka]

On 31.08.23 09:21, Felix Moessbauer wrote:
> This variable is forwarded 1:1 into the container. In case it contains
> a local path (starting with file:///), we issue a warning that this
> path manually needs to be made available inside the container.
>
> Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
> ---
> kas-container | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/kas-container b/kas-container
> index 36784cb..626ed6d 100755
> --- a/kas-container
> +++ b/kas-container
> @@ -504,6 +504,14 @@ if [ -n "${SSTATE_DIR}" ]; then
> -v "$(readlink -fv "${SSTATE_DIR}")":/sstate:rw \
> -e SSTATE_DIR=/sstate
> fi
> +if [ -n "${SSTATE_MIRRORS}" ]; then
> + if echo "${SSTATE_MIRRORS}" | grep -q "file:///"; then
> + echo "Warning: SSTATE_MIRRORS contains a local path." \
> + "Make sure to make this path available inside the container." \
> + >&2

Won't that become annoying when the patch IS available?

Actually, what kas-container normally does is to map that path from the
host into the container. So, either we add such support or we reject
such a setting in SSTATE_MIRRORS.

Jan

> + fi
> + set -- "$@" -e "SSTATE_MIRRORS=${SSTATE_MIRRORS}"
> +fi
>
> if [ -n "${KAS_REPO_REF_DIR}" ]; then
> if [ ! -d "${KAS_REPO_REF_DIR}" ]; then

--

[MOESSBAUER, Felix]

Why? On local hosts, locally available sstate mirrors can be accessed
by just setting SSTATE_DIR (given that RW access is possible). Here,
the SSTATE_MIRRORS variable IMHO is only useful with remote targets
(which are now supported and do not trigger the warning).

In a CI this is different, as the content of the SSTATE_MIRRORS
variable might depend on the runner (some use local-cache, others use
remote cache via s3 or http). But in a CI, the kas image is usually
used directly.

>
> Actually, what kas-container normally does is to map that path from
> the
> host into the container. So, either we add such support or we reject
> such a setting in SSTATE_MIRRORS.

The content of that variable can be quite complex, including multiple
paths. That's why I did not propose support for that in kas-container
in the first versions. In theory, we could try to parse it, mount the
paths inside the container and rewrite the content of the variable, but
this also needs to be tested and maintained. Too much effort IMHO.

For remote targets (e.g. the yocto use-case with pre-built artifacts
from upstream), it makes sense to have it so remote caches work out of
the box. I'm also against rejecting this configuration, as the variable
can contain multiple cache locations and at worst some (e.g. the
locally-mapped ones) are not available but others still work.

For details, see https://wiki.yoctoproject.org/wiki/Enable_sstate_cache

Felix

[Jan Kiszka]

Fine, then just reject file://.

Jan

[MOESSBAUER, Felix]

This is too coarse grained. I propose to keep the implementation as
proposed in v3.

I expect that the warning is shown rather seldom, as local users might
either not use SSTATE_MIRRORS, or use it but just with remote targets.
The only local-build use-case I can think of that uses a local file-
path in this variable is one where the cache is stored on an NFS which
is mounted on the build host.

With the current implementation, the build would still work, but
trigger the warning and (possibly) ignore the cache. However, even in
this case I do not want to bail out, as the user might have explicitly
mounted this folder into the container by using a bind-mount specified
via the --runtime-args option.

In case users complain about the warning, we still can optimize the
implementation. However, as nobody even complained about the missing
SSTATE_MIRRORS support in kas-container, I expect that only very few
people even use that feature in local builds.

Best regards,
Felix

>
> Jan
>

[Jan Kiszka]

Thanks, applied.