Skip to content

codeql: improve Ubuntu dependency installation#16556

Merged
lizan merged 2 commits intoenvoyproxy:mainfrom
jpeach:workflow-error-install-deps
Jun 30, 2021
Merged

codeql: improve Ubuntu dependency installation#16556
lizan merged 2 commits intoenvoyproxy:mainfrom
jpeach:workflow-error-install-deps

Conversation

@jpeach
Copy link
Contributor

@jpeach jpeach commented May 19, 2021

Commit Message:

If apt can't update the package index, it doesn't exit with an error
by default, so add the --error-on=any flag to force an error.

Pass the --yes flag to the install to force non-interactive mode.

Additional Description: This change ought to make the CI error in #16541 more obvious.
Risk Level: Low
Testing: None
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

If apt can't update the package index, it doesn't exit with an error
by default, so add the `--error-on=any` flag to force an error.

Pass the `--yes` flag to the install to force non-interactive mode.

Signed-off-by: James Peach <jpeach@apache.org>
@jpeach
Copy link
Contributor Author

jpeach commented May 19, 2021

CI checks failed with this error:

ERROR: ./docs/root/version_history/current.rst:68: Version history not in alphabetical order (http vs crash support): please check placement of line

which I believe will be fixed by #16555

@phlax
Copy link
Member

phlax commented May 19, 2021

@jpeach its wierd that this didnt fail anyway - on the azure build it also hit this issue but did fail

relatedly im wondering whether we should remove (with sed i guess) the ms packages source - afaiaa its unused in both cases

@jpeach
Copy link
Contributor Author

jpeach commented May 19, 2021

@jpeach its wierd that this didnt fail anyway - on the azure build it also hit this issue but did fail

relatedly im wondering whether we should remove (with sed i guess) the ms packages source - afaiaa its unused in both cases

Yeh, I'm not sure why the other PR gets the failure. It looks like the apt install fails but still exits 0 and I'm not sure why that happens. I think the changes in this PR are a slight improvement, but not sure whether they will fix the issue fully.

@phlax phlax self-assigned this May 19, 2021
@phlax
Copy link
Member

phlax commented May 19, 2021

im not sure that this change is necessary and/or will have any effect

my guess re the reason its not failing is that its not being run -e - if that is the case --error-on wont have any effect

regarding the --yes flag - im kinda surprised that it works normally without this - either way i dont think it will have any bearing on the failure

@jpeach
Copy link
Contributor Author

jpeach commented May 19, 2021

GitHub documents the bash shell as being run with -e.

I think this change is a slight improvement. It's more explicit and passes the right flags to get the behavior we want, but whether it fixes the actual problem seen in other PRs? Probably have to suck it and see :)

Signed-off-by: James Peach <jpeach@apache.org>
@zuercher
Copy link
Member

zuercher commented Jun 9, 2021

@phlax is worth giving this a try?

@phlax
Copy link
Member

phlax commented Jun 9, 2021

is worth giving this a try?

perhaps - the -y flag certainly wont hurt - and i guess neither will the --error-on flag - im just not convinced that it changes anything - i dont want to block it tho

@lizan lizan merged commit 8d0c224 into envoyproxy:main Jun 30, 2021
@jpeach jpeach deleted the workflow-error-install-deps branch July 1, 2021 06:18
baojr added a commit to baojr/envoy that referenced this pull request Jul 1, 2021
* main: (51 commits)
  listener: add filter chain match support for direct source address (envoyproxy#17118)
  Increase common/common coverage (envoyproxy#17193)
  crash_dump: Added local_end_stream_ to crash dump for H2. (envoyproxy#17199)
  codeql: improve Ubuntu dependency installation (envoyproxy#16556)
  ci: Move tooling tests to tooling job (envoyproxy#17071)
  Fix issue with Windows container image (envoyproxy#17113)
  fix filter linking urls (envoyproxy#17185)
  bug fix: fix bug that check_format.py will check files which are ignored (envoyproxy#17195)
  tls: moving the server name into SocketAddressProvider (envoyproxy#16574)
  network: Use std::make_unique and std::make_shared in source/common/network instead of bare new() (envoyproxy#17177)
  Revert "alpha matching: support generic action factory context (envoyproxy#17025)" (envoyproxy#17191)
  ci: Dont clone filter example where not required (envoyproxy#17182)
  alpha matching: support generic action factory context (envoyproxy#17025)
  xds: Clarify comment for RouteMatch.case_sensitive field. (envoyproxy#17176)
  ci: Only publish the required docker image (envoyproxy#17080)
  coverage: fixing flake (envoyproxy#17190)
  api: add cluster_specifier_plugin to RouteAction (envoyproxy#16944)
  wasm: update V8 to v9.2.230.13. (envoyproxy#17183)
  wasm: update Proxy-Wasm C++ Host and SDK to latest (2021-06-24). (envoyproxy#17174)
  owners: add Piotr as senior extension maintainer (envoyproxy#17175)
  ...

Signed-off-by: Garrett Bourg <bourg@squareup.com>
chrisxrepo pushed a commit to chrisxrepo/envoy that referenced this pull request Jul 8, 2021
If apt can't update the package index, it doesn't exit with an error
by default, so add the `--error-on=any` flag to force an error.

Pass the `--yes` flag to the install to force non-interactive mode.

Signed-off-by: James Peach <jpeach@apache.org>
Signed-off-by: chris.xin <xinchuantao@qq.com>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
If apt can't update the package index, it doesn't exit with an error
by default, so add the `--error-on=any` flag to force an error.

Pass the `--yes` flag to the install to force non-interactive mode.

Signed-off-by: James Peach <jpeach@apache.org>
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.

4 participants