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

Do not exit early from platform detection #345

Merged
merged 2 commits into from
Nov 26, 2020
Merged

Do not exit early from platform detection #345

merged 2 commits into from
Nov 26, 2020

Conversation

andrewazores
Copy link
Member

Use single method exit point to ensure post-selection logic (currently, client starting) is performed for both automatic and manual platform selection

Fixes #344

@andrewazores andrewazores requested a review from vic-ma November 25, 2020 17:49
@vic-ma
Copy link
Contributor

vic-ma commented Nov 25, 2020

I'm trying to test the bug on upstream, but I don't see an -e flag in run.sh for the CONTAINER_JFR_PLATFORM env var. Is the environment variable only supposed to be used by the operator? And if it is, can I still test this bug/fix anyway by just adding it to run.sh?

@andrewazores
Copy link
Member Author

andrewazores commented Nov 25, 2020 •
edited
Loading

I was testing against the Operator when I discovered this bug, since the Operator does explicitly set that env var to avoid the cost of doing the automatic detection at startup. That env var could be added to run.sh (I'll add a commit here in a minute to do that), but the only sensible deployment platform to set when running in Docker/Podman is the DefaultPlatformStrategy. Still, manually setting that value vs allowing it to do automatic detection should be enough to trigger this bug. And testing nonsensical platform settings is probably a useful thing to be able to do for testing, too.

@andrewazores andrewazores mentioned this pull request Nov 26, 2020
Merged
vic-ma
vic-ma previously approved these changes Nov 26, 2020
View reviewed changes
@andrewazores
Do not exit early from platform detection
ed21dcb 
Use single method exit point to ensure post-selection logic (currently, client starting) is performed for both automatic and manual platform selection
@andrewazores
Set optional CONTAINER_JFR_PLATFORM env var
e95bd41
@andrewazores
Copy link
Member Author

Rebased.

@andrewazores andrewazores merged commit bbe760b into cryostatio:main Nov 26, 2020
@andrewazores andrewazores deleted the manual-platform-start branch November 26, 2020 19:00
aali309 pushed a commit to aali309/cryostat-legacy that referenced this pull request Jul 22, 2024
@dependabot
build(deps): bump com.nimbusds:nimbus-jose-jwt from 9.31 to 9.37.3 (c…
459d6f3 
…ryostatio#345)

Bumps [com.nimbusds:nimbus-jose-jwt](https://bitbucket.org/connect2id/nimbus-jose-jwt) from 9.31 to 9.37.3.
- [Changelog](https://bitbucket.org/connect2id/nimbus-jose-jwt/src/master/CHANGELOG.txt)
- [Commits](https://bitbucket.org/connect2id/nimbus-jose-jwt/branches/compare/9.37.3..9.31)

---
updated-dependencies:
- dependency-name: com.nimbusds:nimbus-jose-jwt
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vic-ma vic-ma vic-ma approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Manual selection of deployment platform results in PlatformClient not started
2 participants
@andrewazores @vic-ma