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

Update replace existing chrome validation #116

Merged
merged 5 commits into from
Dec 2, 2024

Conversation

marboledacci
Copy link
Contributor

Resolves #57
Update the conditions to remove an existing installation, so it only happens if the version installed is different to the required version.

@marboledacci marboledacci merged commit 22701d0 into main Dec 2, 2024
2 checks passed
@marboledacci marboledacci deleted the feat/adjust-replace-existing-chrome branch December 2, 2024 13:59
@willzoltan
Copy link

@marboledacci Your change here has unfortunately broken our entire Circle CI pipeline.

We use this script in our Dockerfile for circle ci:

FROM cimg/ruby:3.3.6-browsers
...
# Install Chrome tools
# Parameters taken from the source code of the ORB and https://circleci.com/developer/orbs/orb/circleci/browser-tools#commands-install-chrome
ENV ORB_PARAM_CHANNEL=stable
ENV ORB_PARAM_CHROME_VERSION=latest
ENV ORB_PARAM_REPLACE_EXISTING=false
RUN curl -sSL "https://raw.githubusercontent.com/CircleCI-Public/browser-tools-orb/main/src/scripts/install-chrome.sh" | bash

This worked fine for us right up until this PR was merged.

Here is the failing output of our docker build:

#12 [ 7/11] RUN curl -sSL "https://raw.githubusercontent.com/CircleCI-Public/browser-tools-orb/main/src/scripts/install-chrome.sh" | bash
#12 0.070 + curl -sSL https://raw.githubusercontent.com/CircleCI-Public/browser-tools-orb/main/src/scripts/install-chrome.sh
#12 0.071 + bash
#12 0.271 bash: line 5: circleci: command not found
#12 0.278 Google Chrome is not currently installed; installing it
#12 0.283 Preparing Chrome installation for Debian-based systems
#12 0.398 Hit:1 https://download.docker.com/linux/ubuntu focal InRelease
#12 0.480 Hit:2 http://security.ubuntu.com/ubuntu focal-security InRelease
#12 0.520 Hit:3 http://archive.ubuntu.com/ubuntu focal InRelease
#12 0.600 Hit:4 http://archive.ubuntu.com/ubuntu focal-updates InRelease
#12 0.680 Hit:5 http://archive.ubuntu.com/ubuntu focal-backports InRelease
#12 0.877 Reading package lists...
#12 1.900 https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/google-chrome-stable_latest-1_amd64.deb:
#12 1.900 2024-12-09 02:36:04 ERROR 404: Not Found.
#12 1.902 bash: line 157: -2: substring expression < 0
#12 1.903 bash: line 160: google-chrome-stable: command not found
#12 1.905 bash: line 162: google-chrome-stable: command not found
#12 1.907 Google Chrome v (stable) failed to install.
#12 ERROR: process "/bin/bash -exo pipefail -c curl -sSL \"[https://raw.githubusercontent.com/CircleCI-Public/browser-tools-orb/main/src/scripts/install-chrome.sh\](https://raw.githubusercontent.com/CircleCI-Public/browser-tools-orb/main/src/scripts/install-chrome.sh/)" | bash" did not complete successfully: exit code: 1
------
 > [ 7/11] RUN curl -sSL "https://raw.githubusercontent.com/CircleCI-Public/browser-tools-orb/main/src/scripts/install-chrome.sh" | bash:
0.520 Hit:3 http://archive.ubuntu.com/ubuntu focal InRelease
0.600 Hit:4 http://archive.ubuntu.com/ubuntu focal-updates InRelease
0.680 Hit:5 http://archive.ubuntu.com/ubuntu focal-backports InRelease

1.900 https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/google-chrome-stable_latest-1_amd64.deb:
1.900 2024-12-09 02:36:04 ERROR 404: Not Found.
1.902 bash: line 157: -2: substring expression < 0
1.903 bash: line 160: google-chrome-stable: command not found
1.905 bash: line 162: google-chrome-stable: command not found
1.907 Google Chrome v (stable) failed to install.
------

Why is this failing?

Since we don't lave circleci installed in our Dockerfile, that means that the following line

PROCESSED_CHROME_VERSION=$(circleci env subst "$ORB_PARAM_CHROME_VERSION")

Fails with circleci: command not found and just sets PROCESSED_CHROME_VERSION to be empty

Unfortunately your script now fails in that case for installing Google chrome this way.

Potential solution
I noticed that none of the other files in https://github.com/CircleCI-Public/browser-tools-orb/tree/main/src/scripts depend on circleci.

We could change line 5 to handle circleci not being installed?:

if command -v circleci &>/dev/null; then
  # CircleCI is installed, proceed with substitution
  PROCESSED_CHROME_VERSION=$(circleci env subst "$ORB_PARAM_CHROME_VERSION")
else
  # CircleCI is not installed, fallback to using the environment variable as-is
  echo "CircleCI CLI is not installed. Relying on the environment variable ORB_PARAM_CHROME_VERSION to be set manually."
  PROCESSED_CHROME_VERSION=${ORB_PARAM_CHROME_VERSION:-latest} # Default to "latest" if the variable is unset
fi

I've opened a PR #118 and I've tested this with our environment and it works ✅ Please let me know what you think

CC @david-montano-circleci

@marboledacci
Copy link
Contributor Author

marboledacci commented Dec 9, 2024

@willzoltan This is strange, in theory CircleCI is the agent that runs the jobs, so if it was able to run a job you should have that. This usually doesn't need to be manually installed. I'm gonna check this internally to see what is happening, or if the behavior is different when using custom docker images.
At last I will look at your PR, but it shouldn't be required.

@marboledacci
Copy link
Contributor Author

@willzoltan are you using a self hosted runner?

@willzoltan
Copy link

willzoltan commented Dec 9, 2024

@willzoltan are you using a self hosted runner?

@marboledacci Thanks for the quick reply! We're not using self hosted runners, but we do build our own docker images for the pipeline outside of Circle CI. But it should be possible to build the images independent of the circleci agent. The other scripts in src/scripts/ don't depend on the circleci command

@marboledacci
Copy link
Contributor Author

@willzoltan Could you show me the output of the first step of the job, called Spin up environment. It could give me some clue about what is happening.
I have tested the install-chrome in different images, none of them having circleci installed beforehand and it always works.

@willzoltan
Copy link

@willzoltan Could you show me the output of the first step of the job, called Spin up environment. It could give me some clue about what is happening. I have tested the install-chrome in different images, none of them having circleci installed beforehand and it always works.

@marboledacci Here is is. But we build the willzoltan/nearcut:circleci-system_specs-latest images locally and push them to docker hub. And building the image contains this script and it fails because we don't have circleci locally. So we experience the bug locally (resulting in new builds failing) and not on our circle ci jobs.

As a temporary work around, we are using cached docker images of willzoltan/nearcut:circleci-system_specs-latest but until the dependency on circleci is removed (as I propose in #118 ), we are unable to build any new images for Circle CI

Build-agent version 1.0.258435-e071df85 (2024-12-06T15:43:56+0000).
System information:
 Server Version: 24.0.9
 Storage Driver: overlay2
  Backing Filesystem: xfs
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Kernel Version: 5.15.0-1057-aws
 Operating System: Ubuntu 20.04.6 LTS
 OSType: linux
 Architecture: x86_64

Starting container willzoltan/nearcut:circleci-system_specs-latest
  image cache not found on this host, downloading willzoltan/nearcut:circleci-system_specs-latest
circleci-system_specs-latest: Pulling from willzoltan/nearcut
04a5f4cda3ee: Pull complete 
ff496a88c8ed: Pull complete 
0ce83f459fe7: Pull complete 
694198ec8ae2: Pull complete 
45e1216623ed: Pull complete 
96cbba11c330: Pull complete 
a63aa27efd5d: Pull complete 
8a87622d0444: Pull complete 
858d1c101267: Pull complete 
d113c713f70a: Pull complete 
8dbca6e35536: Pull complete 
2b092e0d0c63: Pull complete 
7778251f590f: Pull complete 
e2758caaa0eb: Pull complete 
a62c90d4812b: Pull complete 
6683c12e0ecf: Pull complete 
133a36535ad4: Pull complete 
6dec88084e35: Pull complete 
d0fa3bf3bdd7: Pull complete 
9485bedcd2b0: Pull complete 
65b0999472a9: Pull complete 
1979736175e4: Pull complete 
48580b2dd565: Pull complete 
Digest: sha256:9bfb1761dccb28bbace348fde1f70fc2d0f52826aa1763052446f884c89f0b29
Status: Downloaded newer image for willzoltan/nearcut:circleci-system_specs-latest
willzoltan/nearcut:circleci-system_specs-latest:
  pull stats: download 1.757GiB in 21.527s (83.55MiB/s), extract 1.756GiB in 44.511s (40.41MiB/s)
  time to create container: 13.884s
Starting container cimg/mysql:8.0
Warning: No authentication provided, using CircleCI credentials for pulls from Docker Hub.
  image is cached as cimg/mysql:8.0, but refreshing...
8.0: Pulling from cimg/mysql
Digest: sha256:5dd6bcb841de3a24fdc687d843678c693f261feb4c3c4b54289755384639a839
Status: Image is up to date for cimg/mysql:8.0
cimg/mysql:8.0:
  pull stats: Image was already available so the image was not pulled
  time to create container: 101ms
Starting container cimg/redis:5.0
Warning: No authentication provided, using CircleCI credentials for pulls from Docker Hub.
  image is cached as cimg/redis:5.0, but refreshing...
5.0: Pulling from cimg/redis
Digest: sha256:f7a93fbd03cb043d64d65556c19bfab1939fdfaf29560c26e796a04221fe79d8
Status: Image is up to date for cimg/redis:5.0
cimg/redis:5.0:
  pull stats: Image was already available so the image was not pulled
  time to create container: 51ms
Time to upload agent and config: 349.546009ms
Time to start containers: 445.059443ms

@marboledacci
Copy link
Contributor Author

Sorry, I was not understanding your problem before. I see that your issue is not related to CircleCI execution.
Orbs are designed to run on CircleCI environment and not outside of it, so the behavior you have is not something we have in mind. I will suggest you other solutions and if they don't solve your problems I can take a look at your PR.

  • Install CircleCI CLI on your docker image, you can look how to do it here
  • Use the browserl-tools orb instead of installing chrome on your image. The purpose of the Orb is to allow you to install chrome during the execution of the workflow, you could remove that step from your docker image and add it to the workflow. This would actually help your image to be smaller
  • Use a different method to install chrome on your image.

Keep in mind that even if we merge your PR, it could break again in the future because of the same thing for different parameters or different requirements. The solution you provide is to solve a problem that is outside the scope of an Orb.

I'm guessing you are doing this because of the suggestion here so we are going to look at the caching for this orb.

@willzoltan
Copy link

Thank you for looking into this. We have other reasons why we need to build a docker image ourselves.

And we are using this script entirely within the circle ci ecosystem. Our image is based off the circle ci image cimg/ruby:3.3.6-browsers and then it is run on Circle CI.

I can see that the feature to processed chrome ver with env subst was added in the last couple of weeks. None of the other scripts rely on circleci, and this feature could've been implemented to be compatible with the ORB_PARAM_CHROME_VERSION being generally set in the environment.

It seems like a very reasonable expectation that a bash script should work with the environment variables it depends upon, without necessarily requiring them to come from circleci env subst.

At the very least, why not just default to "latest" when PROCESSED_CHROME_VERSION is empty, rather than it failing? That's just good UX for all users and was the previous behaviour

CC @ryanbourdais

@marboledacci
Copy link
Contributor Author

The usage of the variable ORB_PARAM_CHROME_VERSION is not exclusive with the usage of circleci tool. Both are supposed to work together, when ORB_PARAM_CHROME_VERSION value is another variable, ORB_PARAM_CHROME_VERSION=$ANOTHER_VARIABLE it will be set to the value of that other variable, and when it is not, it will use the value directly.
In general circleci env subst doesn't break any existing behavior, it only adds a new feature to get the value of another environment.
In your case the value is breaking because the tool is not installed, not because the tool is used, that's why I recommend you to install circleci cli before calling the script.

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.

Does replace-existing-chrome option still need to exist?
3 participants