Fix Kubernetes Maven Plugin integration (kubernetes-maven-plugin/it) tests failing on windows

[rohanKanojia]

Component

JKube Kit

Task description

Description

Related to #1338

⚠️ A windows machine is required to reproduce and fix problems in this issue

When executing mvn clean install on Windows, the following unit tests are failing in kubernetes-maven-plugin/it module:

• xml-image
• metadata-labels-annotations
• xml-image-multilayer

You can run a single test in kubernetes-maven-plugin/it module like this:

mvn -Dinvoker.test=xml-image clean install

I think it might have to do with line endings in test expectations \n vs line endings in generated Dockerfile / Kubernetes manifests. If this is the case, we would need to modify ResourceVerify class to ignore difference in line endings while comparing files.

Expected Behavior

Kubernetes Maven Plugin integration tests should pass on windows

Acceptance Criteria

• Kubernetes maven Plugin integration tests pass on windows

[l3002]

Hi @rohanKanojia, I can pick this up, Kindly assign this to me.

Thanks.

[rohanKanojia]

@l3002 : Hello, are you still working on this? If not, is it okay if I unassign you from this issue?

[l3002]

@rohanKanojia : I'm still working on this, I'm having some trouble with the xml-image test in this. I'm trying to access the underlying issue. I'll do my best to send in a PR by the end of next week, if I won't be able to resolve this by then you can unassign me from this issue. I hope you understand.

[rohanKanojia]

Please don't hesitate to ask for help whenever you get stuck.

[l3002]

Sure. Thanks, I'll let you know in case I would need any help with the issue or I won't understand anything.

It is just that for the last few weeks due to some ongoing events in my day job, I wasn't able to pay much attention to all of the issues assigned to me. Moreover, I also have a gig at hand that I need to work on. I just need some more time with the problem. I'm sure I'll be able to resolve it.

Thanks again, I really appreciate all your help.

[l3002]

@rohanKanojia : Just wanted to give you an update, I was able to solve issue with metadata-labels-annotations test, but for the other two, seems to be an issue due to a failure in process creation here:

jkube/jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/ExternalCommand.java

Lines 115 to 123 in bb2e8f7

	private Process startProcess() throws IOException {
	try {
	return Runtime.getRuntime().exec(getArgs(), null, workDir);
	} catch (IOException e) {
	throw new IOException(String.format("Failed to start '%s' : %s",
	getCommandAsString(),
	e.getMessage()), e);
	}
	}

This is invoked to get version for Credential helper, here:

jkube/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/kubernetes/JibBuildService.java

Lines 90 to 91 in bb2e8f7

	final Credential pullRegistryCredential = getRegistryCredentials(
	configuration.getRegistryConfig(), false, pullRegistry);

The issue is that the method mentioned in ExternalCommand aka Runtime.getRuntime().exec(getArgs(), null, workDir) gets passed with a null value for workDir while retrieving version through VersionCommand. This works in Linux as it allows process creation with null value for working directory but in windows as it is unable to find a reference to a working directory, so it leads to an IO Exception and hence, later result, failure of k8s:build goal.

Not Only this but this same issue is actually leading to a build failure for project in windows as well. Here's a screenshot for your reference:

I think we should address those through a new issue. Let me know your thoughts on the same.

As of now, I'm not sure how to solve this problem as it would not be correct to use some other form of exec(String[] cmdarray, String[] envp, File dir) method to create an process in ExternalCommand as other classes which inherit it might need that. For now, I can only think of actually passing the current directory to this exec(String[] cmdarray, String[] envp, File dir) method while calling it through VersionCommand, This way we wouldn't have to replace the exec(), but I haven't tried it yet so can't say, if this will have any other effects or not.

[rohanKanojia]

@l3002 : Could you please create a PR to fix metadata-labels-annotations for now? We can keep this issue open till we fix other two tests.

Not Only this but this same issue is actually leading to a build failure for project in windows as well. Here's a screenshot for your reference

That's quite strange, I'm not able to reproduce this issue on my windows machine. Could you please provide more information about your environment? Maybe your environment settings are affecting test execution (ideally test shouldn't be affected by this)

[l3002]

@rohanKanojia : Sure, I'll create a PR by tomorrow, Won't be able to complete it today, I've already logged off my laptop.

I'll send the environment details tomorrow as well.

[l3002]

@rohanKanojia : Hi Rohan, Sorry for bugging you but I'm having some issue with metadata-labels-annotations as well. I was able to resolve the issue with line breaks. But while validating the kubenetes.yml using the expected\kubenetes.yml. The test still fails the reason behind that is an extra line break character in expected\kubenetes.yml at the end of the value of proxy.istio.io/config.

Here's a screenshot for your reference:

But when I try to find differences between the two files, there is none related to the value of proxy.istio.io/config.

The below screenshot shows the same.

I tried adding a line break at the end of the value in the pom.xml of the test itself, yesterday it was working but today, I tried running the test again for confirmation before creating a PR but It isn't working as expect. I'm not sure why is the line break not being added to the value in the created target file, probably it is getting trimmed and why an extra one is added to the expected value even though both the files have identical values for proxy.istio.io/config.

Could you please try running the test after making the changes in local environment and let me know if the solution works on your local machine?

[l3002]

@rohanKanojia : Do you want me to send you the changes that I made for solving the issue with line breaks?

I made changes to YAML_MAPPER in jkube-kit.common.util.Serialization.

I'm sorry but I haven't worked with integration tests before and therefore, I'm having these issue. I hope I'm not troubling you a lot.

[rohanKanojia]

@l3002 : Let me try to look into this and see what's happening. I'm sorry I didn't investigate the issue properly and marked it as good first issue. Maybe it's a bit involved for contributors.

I hope I'm not troubling you a lot.

Not at all. You're helping us a lot by providing necessary patches and information related to complicated issues like these. There is nothing wrong in asking for help.

[l3002]

I'm sorry I didn't investigate the issue properly and marked it as good first issue.

@rohanKanojia : I actually learned a ton from this issue, as I said I had never worked with an integration test let alone tried to modified one and it has also helped me understand the codebase in a better as well, so it's not a problem at all.

I'll still try to fix this on my own.

As of now, I think that there could be an issue with parsing the text in file in the file to JSON format for validation or maybe while reading the files during verify.groovy is in execution.

And yeah I also wanted to ask you, how exactly does the expected/kubernetes.yml change in line breaks according to the platform, is there something taking care of it?

Not at all. You're helping us a lot by providing necessary patches and information related to complicated issues like these. There is nothing wrong in asking for help.

Thanks a lot, I really appreciate this.

[rohanKanojia]

I tried checking why metadata-labels-annotations test is failing, it's mainly due to how we handle multiline yaml strings here:

jkube/jkube-kit/enricher/api/src/main/java/org/eclipse/jkube/kit/enricher/api/visitor/MetadataVisitor.java

Lines 114 to 118 in 41b05b4

	private String appendTrailingNewLineIfMultiline(String value) {
	if (value.contains(System.lineSeparator()) && !value.endsWith(System.lineSeparator())) {
	return value + System.lineSeparator();
	}
	return value;

Although it's written in platform independent way but we're receiving the annotation value with \n from maven, even if I change code to handle that it doesn't seem to make the test pass. It looks like block chomping indicator is only added when \n is added at the end.

Changing above code to this makes the test pass, but we need to check whether this is something that YAML doesn't support or Jackson is having some problem with adding | when \r\n is present instead of \n.

    if (value.contains("\n") && !value.endsWith("\n")) {
      return value + "\n";
    }

[l3002]

@rohanKanojia : Sorry for the delay in response, I was quite busy for the past two day, I'll check your comments out tomorrow.

Thanks again for helping out.

[l3002]

@rohanKanojia : Okay, I understand it now.

Changing above code to this makes the test pass, but we need to check whether this is something that YAML doesn't support or Jackson is having some problem with adding | when \r\n is present instead of \n.

Should I make these changes and raise a PR or investigate the issue first?

[rohanKanojia]

@l3002 : Sorry for late reply. I think we need to investigate more in this issue. I checked it but couldn't figure out why it's behaving this way. I've removed the good first issue label as it seems a bit involved.

[l3002]

Got it.

[rohanKanojia]

I've created #3014 to fix metadata-labels-annotations integration test. It looks like it does expect \n inside multiline scalar values.

Regarding other two integration test failures xml-image and xml-image-multilayer, these are failing due to differences in line endings of generated Dockerfile. JKube is generating Dockerfile with Line Feed

jkube/jkube-kit/config/image/src/main/java/org/eclipse/jkube/kit/config/image/build/DockerFileKeyword.java

Line 63 in 061c919

sb.append("\n");

If we change this to System.lineSeparator() integration test start failing due to this error :

[INFO] JIB> I/O error for image [registry.access.redhat.com/ubi9/ubi-minimal]:                           
                
[INFO]  
[INFO] JIB>     java.net.SocketException                                                                 
                
[INFO]  
[INFO] JIB>     Connection reset                                                                         
                
[INFO]  
[INFO] JIB> [============                  ] 38.8% complete > pulling platform-specific manifests and con
tainer configs
[INFO] [ERROR] k8s: Failed to execute the build [Error when building JIB image]
[INFO] [INFO] ------------------------------------------------------------------------
[INFO] [INFO] BUILD FAILURE
[INFO] [INFO] ------------------------------------------------------------------------
[INFO] [INFO] Total time:  3.546 s
[INFO] [INFO] Finished at: 2024-05-02T23:43:23+05:30
[INFO] [INFO] ------------------------------------------------------------------------
[INFO] [ERROR] Failed to execute goal org.eclipse.jkube:kubernetes-maven-plugin:1.17-SNAPSHOT:build (defa
ult-cli) on project xml-image-multilayer: Failed to execute the build: Error when building JIB image: Una
ble to build the image tarball: java.net.SocketException: Connection reset -> [Help 1]
[INFO] [ERROR]  
[INFO] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[INFO] [ERROR] Re-run Maven using the -X switch to enable full debug logging.
[INFO] [ERROR]  
[INFO] [ERROR] For more information about the errors and possible solutions, please read the following ar
ticles:
[INFO] [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[INFO] run post-build script verify.groovy
[INFO] Running post-build script: /home/rokumar/work/repos/jkube/kubernetes-maven-plugin/it/target/it/xml
-image-multilayer/verify.groovy
[INFO] java.nio.file.NoSuchFileException: /home/rokumar/work/repos/jkube/kubernetes-maven-plugin/it/targe
t/it/xml-image-multilayer/target/docker/other/simple-image-test/latest/build/Dockerfile

I think Jib build seems to have some problem with this change.

We can stick to converting line separators to \n in ResourceVerify

jkube/jkube-kit/common-it/src/main/java/org/eclipse/jkube/kit/common/ResourceVerify.java

Line 91 in 061c919

return new String(FileCopyUtils.copyToByteArray(Files.newInputStream(path.toPath())), Charset.defaultCharset());

To this:

public static String readFile(File path) throws IOException {
 return new String(FileCopyUtils.copyToByteArray(Files.newInputStream(path.toPath())), Charset.defaultCharset()).replaceAll(System.lineSeparator(), "\n");
}

[rohanKanojia]

#3014 got merged . metadata-labels-annotations test should pass now. Could you please check if you can raise pull request for other two failing tests?

[l3002]

@rohanKanojia : Sure, I'll checkout the other ones tomorrow.