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

feat(nexus3): Allow env vars to be interpolated in INSTALL4J_ADD_VM_PARAMS #827

Merged
merged 3 commits into from
Nov 25, 2023

Conversation

flah00
Copy link
Contributor

@flah00 flah00 commented Nov 16, 2023

Solution

Ensure that the INSTALL4J_ADD_VM_PARAMS environment variable is configured after all others. This allows user defined env vars in Values.env to be interpolated.

Background

I am trying to extract JMX metrics from my Nexus app. We are using datadog. Their advice is to set the pod ip address as an environment variable and then set the JVM RMI hostname as the pod ip.

Kubernetes (or is it helm?) can interpolate environment variables. But the environment variables must be defined before they are used.

WORKS (in pod INSTALL4J_ADD_VM_PARAMS=... -Djava.rmi.server.hostname=1.2.3.4)

  containers:
    - name: foo
      env:
        - name: POD_IP
          valueFrom:
            fieldRef:
              fieldPath: status.podIP
        - name: INSTALL4J_ADD_VM_PARAMS
          value: ... -Djava.rmi.server.hostname=$(POD_IP)

BROKEN (in pod INSTALL4J_ADD_VM_PARAMS=... -Djava.rmi.server.hostname=$(POD_IP))

  containers:
    - name: foo
      env:
        - name: INSTALL4J_ADD_VM_PARAMS
          value: ... -Djava.rmi.server.hostname=$(POD_IP)
        - name: POD_IP
          valueFrom:
            fieldRef:
              fieldPath: status.podIP

Work around

To work around this problem, I currently declare env[].name=POD_IP and then env[].name=JAVA_TOOL_OPTIONS.

But I'd prefer to declare all of my JVM options more centrally.

Copy link
Owner

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

LGTM

@stevehipwell
Copy link
Owner

@flah00 could you please rebase this?

Copy link
Owner

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

LGTM

@stevehipwell
Copy link
Owner

@flah00 sorry, could you rebase again?

Copy link
Owner

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

LGTM

@stevehipwell stevehipwell enabled auto-merge (squash) November 25, 2023 22:24
@stevehipwell stevehipwell merged commit b84a4e5 into stevehipwell:main Nov 25, 2023
1 check passed
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.

2 participants