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.

Sign up for GitHub

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

Jump to bottom

Make lcli docker image portable #5069

Merged
merged 2 commits into from
Jan 30, 2024
Merged

Make lcli docker image portable #5069

merged 2 commits into from
Jan 30, 2024

Conversation

aBMania
Copy link
Contributor

@aBMania aBMania commented Jan 15, 2024

Issue Addressed

Fixes #4370

Proposed Changes

Use FEATURES=portable instead of PORTABLE=true as proposed here

@aBMania aBMania changed the title Unstable Make lcli portable Jan 15, 2024
@CLAassistant
Copy link

CLAassistant commented Jan 15, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@aBMania aBMania changed the title Make lcli portable Make lcli docker image portable Jan 15, 2024
@aBMania aBMania changed the base branch from stable to unstable January 15, 2024 12:52
@chong-he chong-he added ready-for-review The code is ready for review infra-ci labels Jan 24, 2024
dapplion
dapplion approved these changes Jan 29, 2024
View reviewed changes
Copy link
Collaborator

@dapplion dapplion left a comment

Choose a reason for hiding this comment

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

Built and run the updated dockerfile + CI command locally, on an x86 machine tho

.github/workflows/docker.yml
Comment on lines +158 to +163
build-args: |
FEATURES=portable
context: .
push: true
file: ./lcli/Dockerfile
tags: ${{ env.LCLI_IMAGE_NAME }}:${{ env.VERSION }}${{ env.VERSION_SUFFIX }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This syntax is valid for docker/build-push-action and is equivalent to the previous manual command
https://github.com/docker/build-push-action/blob/94d76d3bc1409736cb5dc1ada9502bec3a72973c/__tests__/context.test.ts#L89

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jan 29, 2024
@michaelsproul
Copy link
Member

@Mergifyio queue

@mergify Mergify
Copy link

mergify bot commented Jan 29, 2024 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The pull request #5069 has been manually updated.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@michaelsproul
Copy link
Member

michaelsproul commented Jan 29, 2024 •
edited
Loading

Pushed a new commit to re-run the target branch check

@michaelsproul
Copy link
Member

@Mergifyio refresh

@mergify Mergify
Copy link

mergify bot commented Jan 29, 2024

refresh

✅ Pull request refreshed

@michaelsproul
Copy link
Member

@Mergifyio requeue

@mergify Mergify
Copy link

mergify bot commented Jan 29, 2024

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@michaelsproul
Copy link
Member

@Mergifyio queue

@mergify Mergify
Copy link

mergify bot commented Jan 29, 2024 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 0f345c7

mergify bot added a commit that referenced this pull request Jan 30, 2024
@mergify
Merge of #5069
9deca9a
This was referenced Jan 30, 2024
Closed
Merged
@mergify mergify bot merged commit 0f345c7 into sigp:unstable Jan 30, 2024
28 of 29 checks passed
danielrachi1 pushed a commit to danielrachi1/lighthouse that referenced this pull request Feb 14, 2024
@aBMania @danielrachi1
Make lcli docker image portable (sigp#5069)
467c78d 
* Set lcli docker build to use portable feature (sigp#4370)

* Merge remote-tracking branch 'origin/unstable' into unstable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dapplion dapplion dapplion approved these changes

Assignees
No one assigned
Labels
infra-ci ready-for-merge This PR is ready to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

sigp/lcli container image only built for modern x86 CPUs
5 participants
@aBMania @CLAassistant @michaelsproul @dapplion @chong-he