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

fix(k8s): Endpoints discovery port names/numbers are configurable #1862

Merged
merged 11 commits into from
Feb 12, 2024

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Jan 23, 2024

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Fixes: #1861

Description of the change:

Adds environment variables for customizing the behaviour of the builtin k8s Endpoints discovery mechanism. Rather than always looking for ports named jfr-jmx or numbered 9091, this allows the deploying user to specify a list of acceptable names and a list of acceptable numbers that should be matched, both of them comma-separated. The names list can be set to the empty list by using the value -, and the number list can be set to the empty list by using the value 0. If these variables are not defined or have empty values then they default to the singleton lists of jfr-jmx and 9091 respectively, maintaining the previous behaviour.

Motivation for the change:

For users deploying Cryostat into scenarios where a port named jfr-jmx or numbered 9091 already exists for another purpose, this allows Cryostat to be dropped in with only minor configuration so that the Kubernetes Endpoints discovery mechanism can still be used, but does not mistakenly see non-JMX service ports as connectable targets when they really are not.

How to manually test:

  1. I used helm, ex.: helm install cryostat --set core.image.repository=quay.io/andrewazores/cryostat --set core.image.tag=2.5.0-k8s-discovery-3 --set pvc.enabled=true --set core.route.enabled=true ./charts/cryostat
  2. Then I used the Operator's make sample_app to get a quarkus-test sample application
  3. Then I used oc edit deployment cryostat to set/edit CRYOSTAT_DISCOVERY_K8S_PORT_NAMES and CRYOSTAT_DISCOVERY_K8S_PORT_NUMBERS, and oc edit service quarkus-test to change the port name/number on the sample app.

Copy link
Contributor

Hi @andrewazores! Add at least one of the required labels to this PR

Required labels are : chore,ci,cleanup,docs,feat,fix,perf,refactor,style,test

@andrewazores andrewazores marked this pull request as ready for review January 23, 2024 19:09
@andrewazores
Copy link
Member Author

/build_test

Copy link
Contributor

Workflow started at 1/23/2024, 2:09:59 PM. View Actions Run.

Copy link
Contributor

ARCH IMAGE
amd64 ghcr.io/cryostatio/cryostat:pr-1862-b4591adcd9fca885c19472a4912ea5e74c445a60-linux-amd64
arm64 ghcr.io/cryostatio/cryostat:pr-1862-b4591adcd9fca885c19472a4912ea5e74c445a60-linux-arm64

To run smoketest:

# amd64
CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat:pr-1862-b4591adcd9fca885c19472a4912ea5e74c445a60-linux-amd64 sh smoketest.sh

# or arm64
CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat:pr-1862-b4591adcd9fca885c19472a4912ea5e74c445a60-linux-arm64 sh smoketest.sh

Copy link
Contributor

/build_test completed successfully ✅.
View Actions Run.

@andrewazores
Copy link
Member Author

/build_test

Copy link
Contributor

github-actions bot commented Feb 6, 2024

Workflow started at 2/6/2024, 12:21:03 PM. View Actions Run.

@andrewazores
Copy link
Member Author

/build_test

Copy link
Contributor

github-actions bot commented Feb 6, 2024

Workflow started at 2/6/2024, 12:44:21 PM. View Actions Run.

Copy link
Contributor

github-actions bot commented Feb 6, 2024

ARCH IMAGE
amd64 ghcr.io/cryostatio/cryostat:pr-1862-691a9bda95a3fbc9e611d264435cf0c985632ff0-linux-amd64
arm64 ghcr.io/cryostatio/cryostat:pr-1862-691a9bda95a3fbc9e611d264435cf0c985632ff0-linux-arm64

To run smoketest:

# amd64
CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat:pr-1862-691a9bda95a3fbc9e611d264435cf0c985632ff0-linux-amd64 sh smoketest.sh

# or arm64
CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat:pr-1862-691a9bda95a3fbc9e611d264435cf0c985632ff0-linux-arm64 sh smoketest.sh

Copy link
Contributor

github-actions bot commented Feb 6, 2024

/build_test completed successfully ✅.
View Actions Run.

@andrewazores andrewazores merged commit 838affd into cryostatio:main Feb 12, 2024
8 checks passed
@andrewazores andrewazores deleted the k8s-discovery-config branch February 12, 2024 19:26
mergify bot pushed a commit that referenced this pull request Feb 12, 2024
andrewazores added a commit that referenced this pull request Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] Cryostat should not always assume Kubernetes ports numbered 9091 are JMX
4 participants