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

CUS-349: CI: enable remote execution in jobs that can use it #45

Merged
merged 7 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,46 @@ build:noninteractive --keep_going
build:release --config=noninteractive
build:release --stamp

# Platform-specific options for each supported platform.
build:remote_linux_x64 --extra_execution_platforms=//platform/linux_x64
build:remote_linux_x64 --extra_toolchains=//platform/linux_x64:cc-toolchain
build:remote_linux_x64 --host_platform=//platform/linux_x64
build:remote_linux_x64 --platforms=//platform/linux_x64

build:remote_macos_x64 --extra_execution_platforms=//platform/macos_x64
build:remote_macos_x64 --host_action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1
build:remote_macos_x64 --host_platform=//platform/macos_x64
build:remote_macos_x64 --macos_minimum_os=12
build:remote_macos_x64 --platforms=//platform/macos_x64

build:remote_windows_x64 --extra_execution_platforms=//platform/windows_x64
build:remote_windows_x64 --extra_toolchains=//platform/windows_x64:cc-toolchain
build:remote_windows_x64 --host_platform=//platform/windows_x64
build:remote_windows_x64 --platforms=//platform/windows_x64

# Options for EngFlow remote execution.
build:engflow_common --jobs=40
build:engflow_common --define=EXECUTOR=remote
build:engflow_common --experimental_inmemory_dotd_files
build:engflow_common --experimental_inmemory_jdeps_files
build:engflow_common --incompatible_strict_action_env=true
build:engflow_common --remote_timeout=600
build:engflow_common --legacy_important_outputs=false
build:engflow_common --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1
build:engflow_common --grpc_keepalive_time=30s
build:engflow_common --experimental_remote_cache_compression=true
build:engflow_common --remote_instance_name=auth
build:engflow_common --bes_instance_name=auth
build:engflow_common --bes_lifecycle_events
build:engflow_common --remote_download_minimal

build:engflow_bes --bes_backend=grpcs://opal.cluster.engflow.com
build:engflow_bes --bes_results_url=https://opal.cluster.engflow.com/invocations/auth/
build:engflow_bes --bes_instance_name=auth
build:engflow_bes --bes_lifecycle_events

build:opal --config=engflow_common
build:opal --bes_backend=grpcs://opal.cluster.engflow.com
build:opal --bes_results_url=https://opal.cluster.engflow.com/invocations/auth/
# TODO(CUS-349): enable remote execution
build:engflow --config=engflow_common
build:engflow --config=engflow_bes
build:engflow --remote_executor=grpcs://opal.cluster.engflow.com
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: glass will be faster, because the CI runner is on glass, so it will be faster if the RE action is on the same cluster. Fine to switch in a followup (or create separate configs for glass and opal so that the switch for both CI Runners and RE can be done more easily).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't use Glass yet, mainly because it's not authorized for open source, though I think we can change that. It also doesn't have a Mac RE pool, though we don't strictly need that.

build:engflow --remote_instance_name=auth

# Load authentication flags for the remote service, if any.
# To authenticate with the clusters above, either add flags to this
# .bazelrc.user file or to your $HOME/.bazelrc. These files may contain
# credentials and local file system paths, so they're not checked in.
try-import .bazelrc.user
17 changes: 7 additions & 10 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@ env:
# Recommended here: https://github.com/bazelbuild/bazelisk/issues/88#issuecomment-625178467
BAZELISK_GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
CLUSTER_HOST: opal.cluster.engflow.com
CLUSTER_CONFIG: opal
CRED_HELPER_TOKEN: ${{ secrets.OPAL_CRED_HELPER_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
OPAL_RPC_CREDENTIALS: ${{ secrets.OPAL_RPC_CREDENTIALS }}

jobs:
# TODO(OND-616): use remote execution and caching for all CI Runner jobs.
Expand All @@ -44,7 +42,7 @@ jobs:
- "engflow-pool=ci_sysbox_x64"
- "engflow-runtime=sysbox-runc"
- "engflow-runner-id=${{ github.repository_id }}_bazel-builder_${{ github.run_id }}_${{ github.run_number }}_${{ github.run_attempt }}"
timeout-minutes: 10
timeout-minutes: 30
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this timeout sized, now that it's extended to 30m?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

10m wasn't fast enough to reliably complete a clean build on Opal. It's usually enough, but not always.

I hit a problem that was kind of a worst case: completely cold cache on a pool that had queueing but hadn't scaled up yet. The GoStdLib actions are particularly slow with a cold cache because they're not optimized for RE, and there are a lot of them because we're cross-compiling to every platform on every build.


env:
ARCH: "x64"
Expand All @@ -59,8 +57,7 @@ jobs:
- name: Run all tests
if: success()
run: |
# TODO(CUS-345): Enable remote execution
bazel test --config=noninteractive --config="${CLUSTER_CONFIG}" //...
bazel test --config=noninteractive --config=engflow --config=remote_linux_x64 //...

- name: Log out
run: infra/logout.sh
Expand Down Expand Up @@ -106,8 +103,8 @@ jobs:
shell: bash
run: |
# TODO(CUS-345): Enable remote execution
bazel run --config=noninteractive --config="${CLUSTER_CONFIG}" @rules_go//go -- test ./...
bazel run --config=noninteractive --config="${CLUSTER_CONFIG}" @rules_go//go -- clean -cache -modcache
bazel run --config=noninteractive --config=engflow_bes @rules_go//go -- test ./...
bazel run --config=noninteractive --config=engflow_bes @rules_go//go -- clean -cache -modcache

- name: Log out
shell: bash
Expand Down Expand Up @@ -138,8 +135,8 @@ jobs:
if: success()
run: |
# TODO(CUS-345): Enable remote execution
bazel run --config=noninteractive @rules_go//go -- test ./...
bazel run --config=noninteractive @rules_go//go -- clean -cache -modcache
bazel run --config=noninteractive --config=engflow_bes @rules_go//go -- test ./...
bazel run --config=noninteractive --config=engflow_bes @rules_go//go -- clean -cache -modcache

- name: Log out
run: infra/logout.sh
Expand Down Expand Up @@ -168,7 +165,7 @@ jobs:
- name: Check copyright headers
if: success()
run: |
bazel run --config=noninteractive --config="${CLUSTER_CONFIG}" //infra/internal/check_copyright_headers
bazel run --config=noninteractive --config=engflow_bes //infra/internal/check_copyright_headers

- name: Log out
run: infra/logout.sh
21 changes: 11 additions & 10 deletions .github/workflows/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,10 @@ env:
# Recommended here: https://github.com/bazelbuild/bazelisk/issues/88#issuecomment-625178467
BAZELISK_GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
CLUSTER_HOST: opal.cluster.engflow.com
CLUSTER_CONFIG: opal
CRED_HELPER_TOKEN: ${{ secrets.OPAL_CRED_HELPER_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

jobs:
# TODO(OND-616): use remote execution and caching for all CI Runner jobs.
bazel-builder:
runs-on:
- self-hosted
Expand All @@ -45,7 +43,7 @@ jobs:
- "engflow-pool=ci_sysbox_x64"
- "engflow-runtime=sysbox-runc"
- "engflow-runner-id=${{ github.repository_id }}_bazel-builder_${{ github.run_id }}_${{ github.run_number }}_${{ github.run_attempt }}"
timeout-minutes: 10
timeout-minutes: 30

env:
ARCH: "x64"
Expand All @@ -60,8 +58,11 @@ jobs:
- name: Run all tests
if: success()
run: |
# TODO(CUS-345): Enable remote execution
bazel test --config=noninteractive --config="${CLUSTER_CONFIG}" //...
bazel test \
--config=noninteractive \
--config=engflow \
--config=remote_linux_x64 \
//...

- name: Log out
run: infra/logout.sh
Expand Down Expand Up @@ -107,8 +108,8 @@ jobs:
shell: bash
run: |
# TODO(CUS-345): Enable remote execution
bazel run --config=noninteractive --config="${CLUSTER_CONFIG}" @rules_go//go -- test ./...
bazel run --config=noninteractive --config="${CLUSTER_CONFIG}" @rules_go//go -- clean -cache -modcache
bazel run --config=noninteractive --config=engflow_bes @rules_go//go -- test ./...
bazel run --config=noninteractive --config=engflow_bes @rules_go//go -- clean -cache -modcache

- name: Log out
shell: bash
Expand Down Expand Up @@ -139,8 +140,8 @@ jobs:
if: success()
run: |
# TODO(CUS-345): Enable remote execution
bazel run --config=noninteractive --config="${CLUSTER_CONFIG}" @rules_go//go -- test ./...
bazel run --config=noninteractive --config="${CLUSTER_CONFIG}" @rules_go//go -- clean -cache -modcache
bazel run --config=noninteractive --config=engflow_bes @rules_go//go -- test ./...
bazel run --config=noninteractive --config=engflow_bes @rules_go//go -- clean -cache -modcache

- name: Log out
run: infra/logout.sh
Expand Down Expand Up @@ -169,7 +170,7 @@ jobs:
- name: Check copyright headers
if: success()
run: |
bazel run --config=noninteractive --config="${CLUSTER_CONFIG}" //infra/internal/check_copyright_headers
bazel run --config=noninteractive --config=engflow_bes //infra/internal/check_copyright_headers

- name: Log out
run: infra/logout.sh
9 changes: 8 additions & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ permissions:
contents: write

jobs:
# TODO(OND-616): use remote execution and caching for all CI Runner jobs.
release:
runs-on:
- self-hosted
Expand All @@ -40,8 +39,16 @@ jobs:

steps:
- uses: actions/checkout@v4

- name: Log in
run: infra/login.sh

- name: release
if: success()
run: |
infra/release.sh "${{ inputs.version }}"
env:
GITHUB_TOKEN: ${{ github.token }}

- name: Log out
run: infra/logout.sh
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
bazel-*
.bazelrc.user

# TODO(CUS-389): This file seems to thrash contents between builds (possibly due
# to VSCode extension behavior). Ignore changes until we can better understand
Expand Down
1 change: 0 additions & 1 deletion cmd/engflow_auth/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ go_test(
"//internal/oauthdevice",
"//internal/oauthtoken",
"@com_github_stretchr_testify//assert",
"@com_github_zalando_go_keyring//:go-keyring",
"@org_golang_x_oauth2//:oauth2",
],
)
Expand Down
3 changes: 1 addition & 2 deletions cmd/engflow_auth/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,12 @@ import (
"github.com/EngFlow/auth/internal/oauthdevice"
"github.com/EngFlow/auth/internal/oauthtoken"
"github.com/stretchr/testify/assert"
"github.com/zalando/go-keyring"
"golang.org/x/oauth2"
)

func init() {
// Tests should not interact with the user's real keyring.
keyring.MockInit()
oauthtoken.KeyringMockInit()
}

func codedErrorContains(t *testing.T, gotErr error, code int, wantMsg string) bool {
Expand Down
4 changes: 3 additions & 1 deletion infra/internal/check_copyright_headers/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var (
skipRegexps = []*regexp.Regexp{
regexp.MustCompile(`^go\.mod$`),
regexp.MustCompile(`^go\.sum$`),
regexp.MustCompile(`^LICENSE$`),
regexp.MustCompile(`(^|/)LICENSE$`),
regexp.MustCompile(`^MODULE\.bazel\.lock$`),
regexp.MustCompile(`^MODULE\.bazel$`),
regexp.MustCompile(`^README.md$`),
Expand All @@ -40,6 +40,8 @@ var (
regexp.MustCompile(`^\.bazelversion$`),
regexp.MustCompile(`^\.gitignore$`),
regexp.MustCompile(`\.bzl$`),
regexp.MustCompile(`^platform/linux_x64/builtin_include_directory_paths`),
regexp.MustCompile(`^platform/linux_x64/module.modulemap`),
}
copyrightRegexp = regexp.MustCompile(`^(#|//) Copyright [0-9-]+ EngFlow Inc\.`)
)
Expand Down
2 changes: 2 additions & 0 deletions infra/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ echo "[START] Building artifacts"
BUILD_RELEASE_VERSION="${RELEASE_VERSION}" \
bazel build \
--config=release \
--config=engflow \
--config=remote_linux_x64 \
-- \
//:release_artifacts
echo "[FINISH] Building artifacts"
Expand Down
21 changes: 17 additions & 4 deletions internal/oauthtoken/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ import (
"golang.org/x/oauth2"
)

// KeyringMockInit initializes this package so that its methods may be called
// from a test without reading or writing external state.
func KeyringMockInit() {
mockUsername = "test"
keyring.MockInit()
}

var mockUsername string = ""

type keyringNotFoundError struct {
service string
user string
Expand All @@ -47,12 +56,16 @@ type Keyring struct {
var _ LoadStorer = (*Keyring)(nil)

func NewKeyring() (LoadStorer, error) {
u, err := user.Current()
if err != nil {
return nil, err
username := mockUsername
if username == "" {
u, err := user.Current()
if err != nil {
return nil, err
}
username = u.Username
}
return &Keyring{
username: u.Username,
username: username,
}, nil
}

Expand Down
Loading