Giter VIP home page Giter VIP logo

Comments (28)

l3002 avatar l3002 commented on August 25, 2024 3

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

Thanks.

from jkube.

l3002 avatar l3002 commented on August 25, 2024 2

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.

from jkube.

l3002 avatar l3002 commented on August 25, 2024 1

@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.

from jkube.

l3002 avatar l3002 commented on August 25, 2024 1

@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:

image

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.

image

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?

from jkube.

l3002 avatar l3002 commented on August 25, 2024 1

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

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024 1

@l3002 : Thanks for reporting, I'm able to reproduce this issue via GitHub action. You can create the issue for this failing test.

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

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

from jkube.

l3002 avatar l3002 commented on August 25, 2024

@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.

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

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

from jkube.

l3002 avatar l3002 commented on August 25, 2024

@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:

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:

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:

image

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.

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

@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)

from jkube.

l3002 avatar l3002 commented on August 25, 2024

@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.

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

@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.

from jkube.

l3002 avatar l3002 commented on August 25, 2024

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.

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

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

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";
    }

from jkube.

l3002 avatar l3002 commented on August 25, 2024

@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.

from jkube.

l3002 avatar l3002 commented on August 25, 2024

@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?

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

@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.

from jkube.

l3002 avatar l3002 commented on August 25, 2024

Got it.

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

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
jkubedockerfile-kubernetesmavenplugin-it-failure

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

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");
}

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

#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?

from jkube.

l3002 avatar l3002 commented on August 25, 2024

@rohanKanojia: I made the changes mentioned by you for the other tests in my local environment but it didn't work for me. I ran the test after making the mentioned change but still received the following error.

image

#2841 (comment)

I'm still facing the same issue, the one I mentioned above with the getVersion() method of the CredentialHelper, I'm not sure why am I the only one getting this error.

I've already tried resetting my fork and cloning it again, still the issue persists.

Here are details of my environment:

OS Name - Microsoft Windows 11 Home Single Language
Maven Version - Apache Maven 3.9.6
JDK Version - JDK 17

from jkube.

l3002 avatar l3002 commented on August 25, 2024

I have even tried updating my JDK version to 21 but still the same error.

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

I made the changes mentioned by you for the other tests in my local environment but it didn't work for me. I ran the test after making the mentioned change but still received the following error.

@l3002 : Hello, Could you please share the changes you performed and command you used to run the test?

I have even tried updating my JDK version to 21 but still the same error.

It might be possible that you didn't compile the common-it/ module after performing the changes. I followed these steps and test were passing after making these changes:

  1. Update the readFile() method in ResourceVerify
  2. Perform mvn clean install in jkube-kit/common-it directory
  3. Run the failing tests again using mvn -Dinvoker.test=xml-image-multilayer clean install

from jkube.

l3002 avatar l3002 commented on August 25, 2024

@rohanKanojia : Hi, I'm actually doing the same that you mentioned above but still I do face the same issue with the integration test. The only difference between the method mentioned by you and mine is that, I actually use mvn -DskipTests clean install so that any failing tests are skipped.

But just to check, I used mvn clean install and still I'm getting the same error.

If you want I can create a PR with for the changes mentioned by you.

from jkube.

rohanKanojia avatar rohanKanojia commented on August 25, 2024

@l3002 : Yes, that would really help. Please go ahead and create a pull request.

from jkube.

l3002 avatar l3002 commented on August 25, 2024

@rohanKanojia: I'll check what's the issue at my end. Also, after you made the changes for metadata-annotation test, another unit test has started to fail. I'm not sure if it's just a problem with my local environment, but if you could please check and let me know if the test is failing, then i can raise an issue and work on it.

image

from jkube.

l3002 avatar l3002 commented on August 25, 2024

For now, I've raised a PR with the mentioned changes.

from jkube.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.