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

Incorrect warning about overriding environment variable #887

Closed
rgordill opened this issue Sep 6, 2021 · 1 comment · Fixed by #1108
Closed

Incorrect warning about overriding environment variable #887

rgordill opened this issue Sep 6, 2021 · 1 comment · Fixed by #1108
Assignees
Labels
bug Something isn't working

Comments

@rgordill
Copy link

rgordill commented Sep 6, 2021

Description

Info

  • Eclipse JKube version : 1.4.0
  • Maven version (mvn -v) :
Maven home: /usr/share/maven
Java version: 11.0.12, vendor: Red Hat, Inc., runtime: /usr/lib/jvm/java-11-openjdk-11.0.12.0.7-0.fc34.x86_64
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.13.12-200.fc34.x86_64", arch: "amd64", family: "unix"
  • Kubernetes / Red Hat OpenShift setup and version : k8s 1.21

  • If it's a bug, how to reproduce : Just adding images with environment vars that can be overriden to resources. Ex.

<configuration>
	<images>
		<image>
			<name>quay.io/example/%a:%l</name>
			<build>
				<from>quay.io/jkube/jkube-java-binary-s2i:0.0.9</from>
				<assembly>
					<targetDir>/deployments</targetDir>
				</assembly>
			</build>
		</image>
	</images>
	<resources>
		<env>
			<!-- disable jolokia -->
			<AB_JOLOKIA_OFF>true</AB_JOLOKIA_OFF>
			<JAVA_OPTIONS>-Djgroups.dns.query=${project.artifactId}-ping</JAVA_OPTIONS>
			<JAVA_ARGS>--spring.profiles.active=${deployment.mode},${infinispan.mode} --spring.application.name=customer-service-cache --spring.cloud.kubernetes.config.name=customer-service-cache-config --spring.cloud.kuberentes.config.paths=/opt/jboss/etc --spring.cloud.kubernetes.config.enableApi=false</JAVA_ARGS>
		</env>
	</resources>
</configuration>
  • Log:
[INFO] k8s: jkube-controller: Adding a default Deployment
[WARNING] k8s: jkube-image: Environment variable AB_JOLOKIA_OFF will not be overridden: trying to set the value true, but its actual value is true
[WARNING] k8s: jkube-image: Environment variable JAVA_ARGS will not be overridden: trying to set the value --spring.profiles.active=kubernetes,embedded --spring.application.name=customer-service-cache --spring.cloud.kubernetes.config.name=customer-service-cache-config --spring.cloud.kuberentes.config.paths=/opt/jboss/etc --spring.cloud.kubernetes.config.enableApi=false, but its actual value is --spring.profiles.active=kubernetes,embedded --spring.application.name=customer-service-cache --spring.cloud.kubernetes.config.name=customer-service-cache-config --spring.cloud.kuberentes.config.paths=/opt/jboss/etc --spring.cloud.kubernetes.config.enableApi=false
[WARNING] k8s: jkube-image: Environment variable JAVA_OPTIONS will not be overridden: trying to set the value -Djgroups.dns.query=customer-service-cache-ping, but its actual value is -Djgroups.dns.query=customer-service-cache-ping
@manusa manusa added the bug Something isn't working label Sep 27, 2021
@rohanKanojia rohanKanojia self-assigned this Nov 8, 2021
@rohanKanojia
Copy link
Member

I think this warning is coming from here:
https://github.com/eclipse/jkube/blob/40b056179d70d28166b58ca01e15393e3fc73a22/jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/ImageEnricher.java#L286-L290

We check whether we already have the environment variable with the provided name. If absent, we're adding it to containerEnvVars otherwise we're logging Environment variable will not be overridden message without checking whether old and new values are different or not. I think we should add a condition here to log only when provided value and old value are different. Something like this should work:

EnvVar oldEnvVar = containerEnvVars.stream()
  .filter(e -> e.getName().equals(newEnvVar.getName()))
  .findFirst().orElse(null);
if (oldEnvVar == null) {
    containerEnvVars.add(newEnvVar);
} else if (!newEnvVar.getValue().equals(oldEnvVar.getValue())) {
    log.warn(
      "Environment variable %s will not be overridden: trying to set the value %s, but its actual value is %s",
      newEnvVar.getName(), newEnvVar.getValue(), oldEnvVar.getValue());
}

rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Nov 8, 2021
… variable

Don't log warning about not overriding environment variable when both
old and new values are same.

Signed-off-by: Rohan Kumar <[email protected]>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Nov 9, 2021
… variable

Don't log warning about not overriding environment variable when both
old and new values are same.

Signed-off-by: Rohan Kumar <[email protected]>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Nov 11, 2021
… variable

Don't log warning about not overriding environment variable when both
old and new values are same.

Signed-off-by: Rohan Kumar <[email protected]>
manusa pushed a commit that referenced this issue Nov 11, 2021
Don't log warning about not overriding environment variable when both
old and new values are same.

Signed-off-by: Rohan Kumar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants