bazel/ci: Improve flags/config #28856
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
phlax
requested review from
RyanTheOptimist,
alyssawilk and
abeyad
as code owners
phlax
force-pushed
the
bazel-flags-rbe
branch
4 times, most recently
from
347e6be to
7d4b193
Compare
phlax
commented
Aug 5, 2023
phlax
force-pushed
the
bazel-flags-rbe
branch
2 times, most recently
from
bf4251c to
7351e87
Compare
phlax
changed the title
phlax
changed the title
phlax
force-pushed
the
bazel-flags-rbe
branch
9 times, most recently
from
d087630 to
e60ba77
Compare
phlax
commented
Aug 6, 2023
phlax
force-pushed
the
bazel-flags-rbe
branch
2 times, most recently
from
496d7e7 to
d6c7afd
Compare
phlax
commented
Aug 6, 2023
phlax
force-pushed
the
bazel-flags-rbe
branch
4 times, most recently
from
6b5da9b to
16d6b0a
Compare
phlax
force-pushed
the
bazel-flags-rbe
branch
4 times, most recently
from
4a81645 to
13bbd4d
Compare
phlax
changed the title
Signed-off-by: Ryan Northey <[email protected]>
|
this PR is quite invasive but has been extensively tested, fixes a stack of problems and lays the groundwork to fix a stack more |
|
Sending this to @RyanTheOptimist @keith due to the Envoy Mobile implications. |
phlax
commented
Aug 6, 2023
|
Does this impact the current ability of failed mobile job to be restarted (which doesn't seem to be available for non-mobile jobs) |
not at all - i havent touched the job handling at all in this PR |
RyanTheOptimist
approved these changes
Aug 7, 2023
| @@ -40,7 +40,7 @@ Do not allow any bots or app users to do so, unless this is specifically require | |||
| For example, you could add a `job` condition to prevent any bots from triggering the workflow: | |||
|
|
|||
| ```yaml | |||
| if: | | |||
| if: >- | |||
There was a problem hiding this comment.
OOC, what does >- mean here?
There was a problem hiding this comment.
strip ws - previously this condition was always returning true as false ~= " "
thanks - there are a few things (eg re configs) that should hopefully become clearer with subsequent prs - i will monitor this closely going through, and have some behind to rebase and push forward also |
phlax
added a commit
to phlax/envoy
that referenced
this pull request
Aug 8, 2023
Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: phlax <[email protected]>
phlax
added a commit
to phlax/envoy
that referenced
this pull request
Aug 8, 2023
Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: phlax <[email protected]>
abeyad
added a commit
to abeyad/envoy
that referenced
this pull request
Aug 14, 2023
The mobile Bazel options were updated in envoyproxy#28856, but the mobile_release workflow wasn't updated to use mobile-release-android. It was probably not caught by CI because it is a manual workflow that is run once a week as opposed to running on every PR. Signed-off-by: Ali Beyad <[email protected]>
abeyad
added a commit
to abeyad/envoy
that referenced
this pull request
Aug 14, 2023
The mobile Bazel options were updated in envoyproxy#28856, but the mobile_release workflow wasn't updated to use mobile-release-android. It was probably not caught by CI because it is a manual workflow that is run once a week as opposed to running on every PR. Signed-off-by: Ali Beyad <[email protected]>
abeyad
added a commit
that referenced
this pull request
Aug 14, 2023
#29009) The mobile Bazel options were updated in #28856, but the mobile_release workflow wasn't updated to use mobile-release-android. It was probably not caught by CI because it is a scheduled workflow that is run once a week as opposed to running on every PR. Signed-off-by: Ali Beyad <[email protected]>
phlax
added a commit
that referenced
this pull request
Aug 21, 2023
Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: phlax <[email protected]>
phlax
added a commit
that referenced
this pull request
Aug 22, 2023
Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: phlax <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently our bazel flags are not very well structured
the problem is 10x worse in mobile, which also hijacks quite a few flags from envoys config
this rinses out duplicates in the config, separates mobile config to its own ~namespace and gets rid of related warnings
this also quietens mobile CI and standardizes the RBE config which will allow adding more complex setups (#28814 )
also removes github mobile token condition that would never be hit and tightens workflow permissions generally
also separates github workflow diskspace cleanup code to an action
and switches default (mostly) for remote exec to
--remote_download_minimal, updating any ci that requires more downloads, runfiles, etcmore generally moves/consolidates config to the .bazelrc file to make it easier to manage them
broadly, its some first steps to addressing the issues posted above in terms of separating responsibility
Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]