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

[build] Add DOCKER_USER_ENV option #16904

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jusherma
Copy link
Contributor

Why I did it

To enable users to pass environmental variables into the SONiC build containers, especially values that cannot be safely written to disk, like authentication tokens.

The DOCKER_USER_ENV parameter supports any format allowed by Docker's -e flag. Values can be set explicitly or inherited from the host environment. For example, both of these are valid:

$ make DOCKER_USER_ENV="A=foo,B=bar" sonic-slave-bash

$ export A=foo
$ export B=bar
$ make DOCKER_USER_ENV="A,B" sonic-slave-bash
Work item tracking
  • Microsoft ADO (number only):

How I did it

The optional Makefile parameter, DOCKER_USER_ENV, accepts comma-separated values to be passed as -e arguments to the $(DOCKER_RUN) call. This is the environmental variable equivalent of the DOCKER_BUILDER_USER_MOUNT parameter.

How to verify it

Build the sonic-slave-bash pseudotarget and check the environmental variables set inside of the build container(s):

  1. Passing values explicitly:
$ make DOCKER_USER_ENV="A=foo,B=bar" sonic-slave-bash

BLDENV=buster make -f Makefile.work sonic-slave-bash
jusherma@630c70f71b19:/sonic$ echo ${A} ${B}
foo bar

BLDENV=bullseye make -f Makefile.work sonic-slave-bash
jusherma@02e2af7ed76c:/sonic$ echo ${A} ${B}
foo bar
  1. Inheriting values from the host environment:
$ export A=foo
$ export B=bar
$ make DOCKER_USER_ENV="A,B" sonic-slave-bash

BLDENV=buster make -f Makefile.work sonic-slave-bash
jusherma@6a7ffcba48c5:/sonic$ echo ${A} ${B}
foo bar

BLDENV=bullseye make -f Makefile.work sonic-slave-bash
jusherma@e6a22c6175d4:/sonic$ echo ${A} ${B}
foo bar

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Enable users to pass environmental variables into the SONiC build containers

Link to config_db schema for YANG module changes

n/A

A picture of a cute animal (not mandatory but encouraged)

@jusherma
Copy link
Contributor Author

/azp help

Copy link

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@jusherma
Copy link
Contributor Author

/azp run

Copy link

Commenter does not have sufficient privileges for PR 16904 in repo sonic-net/sonic-buildimage

@kevinskwang kevinskwang requested a review from saiarcot895 March 20, 2024 03:15
@kevinskwang
Copy link
Contributor

/azp run

Copy link

Commenter does not have sufficient privileges for PR 16904 in repo sonic-net/sonic-buildimage

@@ -348,6 +348,11 @@ ifneq ($(SIGNING_CERT),)
endif
endif

# Allow user-defined environmental variables to be imported into the build containers
ifneq ($(DOCKER_USER_ENV),)
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about using SONIC_BUILDER_EXTRA_CMDLINE to add the additional options?

$(SONIC_BUILDER_EXTRA_CMDLINE)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it looks like SONIC_BUILDER_EXTRA_CMDLINE works for our use case. You can close this PR if you don't think it adds further value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants