10000 DEV-510: Fix availability of CI env var on remote executions by ifbyol · Pull Request #4545 · okteto/okteto · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

DEV-510: Fix availability of CI env var on remote executions #4545

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

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

ifbyol
Copy link
Member
@ifbyol ifbyol commented Oct 29, 2024

Proposed changes

Fixes DEV-510

Nicklas reported some time ago that the CI env var wasn't being propagated correctly to the test execution in remote. I was taking a look at it today as it also affected to env var OKTETO_IS_PREVIEW_ENVIRONMENT. After some testing, I saw that is related to using ARG instead of ENV. I'm not sure why, but using ARG with default value is not making that env var available in remote-run command. The only way I had to make it available was:

  • Passing the build arg with the value here
  • Using ENV instead of ARG

As there is not final image built, which is the main difference between ENV and ARG (the availability of that var in the running container or not), I opted by switching it to use ENV

How to validate

  • Build the image from this branch
  • Export in your terminal the variables CI and OKTETO_IS_PREVIEW_ENVIRONMENT. export CI=true and export OKTETO_IS_PREVIEW_ENVIRONMENT=true
  • Then, prepare an Okteto manifest like the following one:
test:
  unit:
    commands:
      - echo $CI
      - echo $OKTETO_IS_PREVIEW_ENVIRONMENT
  • If you run okteto test unit with the CLI 3.0, you will see how nothing is printed
  • If you run it with the version of this branch, you will see how both values are printed

CLI Quality Reminders 🔧

For both authors and reviewers:

  • Scrutinize for potential regressions
  • Ensure key automated tests are in place
  • Build the CLI and test using the validation steps
  • Assess Developer Experience impact (log messages, performances, etc)
  • If too broad, consider breaking into smaller PRs
  • Adhere to our code style and code review guidelines

Signed-off-by: Nacho Fuertes <nacho@okteto.com>
@ifbyol ifbyol added release/bug-fix run-e2e When used on a PR run windows & unix e2e labels Oct 29, 2024
@ifbyol ifbyol requested a review from a team as a code owner October 29, 2024 19:58
Copy link
codecov bot commented Oct 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.16%. Comparing base (e225c4a) to head (f67818b).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4545   +/-   ##
=======================================
  Coverage   46.16%   46.16%           
=======================================
  Files         355      355           
  Lines       28393    28393           
=======================================
  Hits        13109    13109           
  Misses      14173    14173           
  Partials     1111     1111           

Copy link
Contributor
@teresaromero teresaromero left a comment

Choose a reason for hiding this comment

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

was curious and read about this here, in case helps someonelse undertand the issue https://vsupalov.com/docker-arg-env-variable-guide/

@@ -86,7 +86,7 @@ WORKDIR /okteto/src
ENV {{$key}}={{$val}}
{{end}}
{{range $key, $val := .OktetoCommandSpecificEnvVars }}
ARG {{$key}} {{$val}}
ENV {{$key}}={{$val}}
Copy link
Contributor

Choose a reason for hiding this comment

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

ARG {{$key}}={{$val}} also works. The main difference between ARG and ENV is that you can change the value of ARG for different builds. Since we autogenerate these dockerfiles, instead of passing ARG values for the same dockerfile, both options are equivalent.

@ifbyol ifbyol added the backport release-3.9 Backport this PR to CLI versions 3.9 label Nov 4, 2024
@ifbyol ifbyol merged commit ce2c34f into master Nov 4, 2024
37 checks passed
@ifbyol ifbyol deleted the ifbyol/make-ci-env-var-available branch November 4, 2024 10:55
github-actions bot pushed a commit that referenced this pull request Nov 4, 2024
Signed-off-by: Nacho Fuertes <nacho@okteto.com>
(cherry picked from commit ce2c34f)
ifbyol added a commit that referenced this pull request Nov 4, 2024
…4551)

Signed-off-by: Nacho Fuertes <nacho@okteto.com>
(cherry picked from commit ce2c34f)

Co-authored-by: Ignacio Fuertes <nacho@okteto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-3.9 Backport this PR to CLI versions 3.9 release/bug-fix run-e2e When used on a PR run windows & unix e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0