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

Remove aws cli install. #110972

Merged
merged 2 commits into from
May 6, 2023
Merged

Remove aws cli install. #110972

merged 2 commits into from
May 6, 2023

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Apr 29, 2023

All runner images have the AWS CLI 2 installed, so there isn't a really strong reason to install our own version anymore.

The version we were installing was 1.27.122. The runner images currently have 2.11.x (the exact version varies by image).

I do not have the means to really test if the new version has any issues. I looked at all the aws commands, and none of them seem to be doing anything unusual. The page at https://docs.aws.amazon.com/cli/latest/userguide/cliv2-migration-changes.html contains a list of all the breaking changes, and I didn't see anything that looked important.

@rustbot
Copy link
Collaborator

rustbot commented Apr 29, 2023

r? @pietroalbini

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Apr 29, 2023
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

r? @Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Apr 30, 2023

📌 Commit 7cb798c has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 30, 2023
@bors
Copy link
Contributor

bors commented Apr 30, 2023

⌛ Testing commit 7cb798c with merge 200be0aeacba7e41d86347224b22163f6cb8b696...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Apr 30, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 30, 2023
@ehuss
Copy link
Contributor Author

ehuss commented Apr 30, 2023

The best I can tell from searching around is that error is generated because the v2 CLI now uses the IMDS service to determine the region, but for some reason it doesn't work on GitHub Actions. The solution is to specify the region so that it does not have to go discover it.

An alternate solution is possibly to set AWS_EC2_METADATA_DISABLED, which disables the IMDS lookup. I believe it still initially contacts the "global" region and then gets redirected to the correct region. Setting the region avoids that global lookup and redirect. I can try that solution as an alternate if you'd prefer to not hard-code the region.

Also, let me know if you would like the ability to set the region separately for artifacts versus caches.

From what I can tell, the buckets currently used in CI are rust-lang-ci-sccache2, rust-lang-ci-mirrors, and rust-lang-ci2.

I haven't tested on GitHub Actions if this actually solves the problem. I can do some testing on my own account if you would prefer to not blindly try things.

@pietroalbini
Copy link
Member

Setting the AWS region should be enough.

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented May 2, 2023

📌 Commit 291b61e has been approved by pietroalbini

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 2, 2023
@bors
Copy link
Contributor

bors commented May 3, 2023

⌛ Testing commit 291b61e with merge da5bacec5e1c48ed5b0193eb91e7ffc6a05f1d11...

@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
drwxrwxrwt 14 root root 4.0K May  3 19:16 ..
-rw-r--r--  1 gha  gha  125K May  3 19:16 cpu-aarch64-gnu.csv
-rw-r--r--  1 gha  gha  2.8M May  3 19:16 metrics-aarch64-gnu.json

Attempting with retry: aws s3 cp --storage-class INTELLIGENT_TIERING --no-progress --recursive --acl public-read /tmp/tmp.e92jmXDLmp s3://rust-lang-ci2/rustc-builds/da5bacec5e1c48ed5b0193eb91e7ffc6a05f1d11
/gha/_work/rust/rust/src/ci/scripts/../shared.sh: line 17: aws: command not found
/gha/_work/rust/rust/src/ci/scripts/../shared.sh: line 17: aws: command not found
Command failed. Attempt 3/5:
/gha/_work/rust/rust/src/ci/scripts/../shared.sh: line 17: aws: command not found
Command failed. Attempt 4/5:

@bors
Copy link
Contributor

bors commented May 3, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 3, 2023
@ehuss
Copy link
Contributor Author

ehuss commented May 3, 2023

I'm not too familiar with the self-hosted runner. I forgot that it has its own environment. Is there a separate place where its environment is defined? Or should the CI script just manually install the aws cli for that runner?

@ehuss
Copy link
Contributor Author

ehuss commented May 6, 2023

Thanks for the pointers! I have posted rust-lang/gha-self-hosted#18.

@Mark-Simulacrum
Copy link
Member

@bors retry

Rebuilt the self hosted image with the patch above, so hopefully this should merge now.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 6, 2023
@bors
Copy link
Contributor

bors commented May 6, 2023

⌛ Testing commit 291b61e with merge a77c552...

@bors
Copy link
Contributor

bors commented May 6, 2023

☀️ Test successful - checks-actions
Approved by: pietroalbini
Pushing a77c552 to master...

1 similar comment
@bors
Copy link
Contributor

bors commented May 6, 2023

☀️ Test successful - checks-actions
Approved by: pietroalbini
Pushing a77c552 to master...

@bors bors added merged-by-bors This PR was explicitly merged by bors. labels May 6, 2023
@bors bors merged commit a77c552 into rust-lang:master May 6, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 6, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a77c552): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.7% [-2.8%, -0.7%] 4
Improvements ✅
(secondary)
-5.2% [-7.5%, -0.2%] 7
All ❌✅ (primary) -1.7% [-2.8%, -0.7%] 4

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.2% [3.2%, 3.2%] 1
Improvements ✅
(primary)
-1.4% [-2.7%, -0.1%] 2
Improvements ✅
(secondary)
-2.3% [-2.4%, -2.1%] 4
All ❌✅ (primary) -1.4% [-2.7%, -0.1%] 2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 653.481s -> 654.347s (0.13%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants