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

feat(agent): add pod mutation webhook to inject agent #991

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat(agent): add pod mutation webhook to inject agent #991

wants to merge 11 commits into from

Conversation

ebaron
Copy link
Member

@ebaron ebaron commented Dec 20, 2024 •
edited
Loading

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: git commit -S -m "YOUR_COMMIT_MESSAGE"

Fixes: #970

Description of the change:

  • Adds a mutating admission webhook for pods that responds to a pair of labels by injecting the Cryostat agent and configuring it to work with the specified Cryostat instance.

Motivation for the change:

  • Makes it much easier to set up the Cryostat agent with arbitrary pods.

How to manually test:

  1. make deploy
  2. make create_cryostat_cr
  3. make sample_app
  4. Modify the sample app deployment as follows:
    1. Override JAVA_OPTS_APPEND to not inject the built-in Cryostat agent. 
      - env:
  - name: JAVA_OPTS_APPEND
    value: -Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager -Dio.cryostat.agent.shaded.org.slf4j.simpleLogger.defaultLogLevel=debug
    2. Add the following labels: 
      cryostat.io/name: cryostat-sample
cryostat.io/namespace: cryostat-operator-system
    3. I had to also increase the memory limit to prevent OOM.
  5. The sample application should appear as an agent target in Cryostat

I've filed issues for the TODOs in this PR. There are a few of them, but they're all pretty small compared to this PR:
#992, #993, #994, #995, #996

@ebaron ebaron added the feat New feature or request label Dec 20, 2024
@mergify mergify bot added the safe-to-test label Dec 20, 2024
@ebaron ebaron changed the title Pod mutation feat(agent): add pod mutation webhook to inject agent Dec 20, 2024
@andrewazores
Copy link
Member

I added a config sample to do DEPLOY_NAMESPACE=$(oc project -q) make sample_app_agent_injected for test convenience. Seems to work nicely!

@ebaron ebaron marked this pull request as ready for review December 20, 2024 22:15
@ebaron ebaron requested review from andrewazores and a team December 20, 2024 22:15
@ebaron
Copy link
Member Author

ebaron commented Dec 20, 2024

/build_test

ebaron
ebaron commented Dec 20, 2024
View reviewed changes
internal/controllers/common/common_utils.go
prefixWithKind := strings.ToLower(gvk.Kind)
if short {
// Prefix is 5 characters: 4 from kind, and hyphen
prefixWithKind = prefixWithKind[:4] // TODO can we use a weaker and shorter hash and keep the full kind?
Copy link
Member Author

Choose a reason for hiding this comment

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

@andrewazores what do you think about using a non-cryptographic hash function for this instead?
With https://pkg.go.dev/hash/fnv, we could use:
cryostat-<128-bit hash> instead of cryo-<224-bit hash>

@github-actions GitHub Actions
Copy link

/build_test completed successfully ✅.
View Actions Run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewazores andrewazores Awaiting requested review from andrewazores

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
feat New feature or request safe-to-test
Projects
4.0.0 release
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Story] Mutate pods to inject and configure agent
2 participants
@ebaron @andrewazores