GoogleContainerTools / jib

🏗 Build container images for your Java applications.
Apache License 2.0
13.66k stars 1.44k forks source link

Kokoro Windows continuous job failing after image migration #3978

Closed mpeddada1 closed 1 year ago

mpeddada1 commented 1 year ago

Error Message:

ERROR: JAVA_HOME is set to an invalid directory: c:\program files\java\jdk1.8.0_152

Please set the JAVA_HOME variable in your environment to match the
location of your Java installation.
emmileaf commented 1 year ago

3979 works past error message above, but with additional issue in test suite:

com.google.cloud.tools.jib.cli.buildfile.BuildFilesTest > testToBuildFileSpec_templateMultiLineBehavior FAILED
    java.lang.IllegalArgumentException: Cannot resolve variable 'replace
    this' (enableSubstitutionInVariables=false).
        at org.apache.commons.text.StringSubstitutor.substitute(StringSubstitutor.java:1532)
        at org.apache.commons.text.StringSubstitutor.substitute(StringSubstitutor.java:1389)
        at org.apache.commons.text.StringSubstitutor.replaceIn(StringSubstitutor.java:1121)
        at org.apache.commons.text.io.StringSubstitutorReader.read(StringSubstitutorReader.java:298)
        at org.yaml.snakeyaml.reader.StreamReader.update(StreamReader.java:176)
        at org.yaml.snakeyaml.reader.StreamReader.ensureEnoughData(StreamReader.java:169)
        at org.yaml.snakeyaml.reader.StreamReader.ensureEnoughData(StreamReader.java:164)
        at org.yaml.snakeyaml.reader.StreamReader.peek(StreamReader.java:119)
        at org.yaml.snakeyaml.scanner.ScannerImpl.scanToNextToken(ScannerImpl.java:1229)
        at org.yaml.snakeyaml.scanner.ScannerImpl.fetchMoreTokens(ScannerImpl.java:345)
        at org.yaml.snakeyaml.scanner.ScannerImpl.checkToken(ScannerImpl.java:263)
        at org.yaml.snakeyaml.parser.ParserImpl$ParseImplicitDocumentStart.produce(ParserImpl.java:235)
        at org.yaml.snakeyaml.parser.ParserImpl.peekEvent(ParserImpl.java:185)
        at org.yaml.snakeyaml.parser.ParserImpl.getEvent(ParserImpl.java:195)
        at com.fasterxml.jackson.dataformat.yaml.YAMLParser.nextToken(YAMLParser.java:418)
        at com.fasterxml.jackson.databind.ObjectMapper._initForReading(ObjectMapper.java:4817)
        at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4723)
        at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3690)
        at com.google.cloud.tools.jib.cli.buildfile.BuildFiles.toBuildFileSpec(BuildFiles.java:55)
        at com.google.cloud.tools.jib.cli.buildfile.BuildFiles.toJibContainerBuilder(BuildFiles.java:81)
        at com.google.cloud.tools.jib.cli.buildfile.BuildFilesTest.testToBuildFileSpec_templateMultiLineBehavior(BuildFilesTest.java:189)

Follow-up:

Using the unix-based newline instead of System.lineSeparator() in the test gets past this error, https://github.com/GoogleContainerTools/jib/blob/fa8cc958f4ddad55a9eba043fdfd7c32eddb49b6/jib-cli/src/test/java/com/google/cloud/tools/jib/cli/buildfile/BuildFilesTest.java#L187

I suspect this is because UTF-8 is used when reading the buildfile for this test, but the default charset in the new Kokoro windows environment is now Windows-1252 (and with a different line separator). https://github.com/GoogleContainerTools/jib/blob/fa8cc958f4ddad55a9eba043fdfd7c32eddb49b6/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/buildfile/BuildFiles.java#L53-L54

emmileaf commented 1 year ago

Additional errors in later tests (possibly also related to charset discrepancy?):

com.google.cloud.tools.jib.plugins.common.UpdateCheckerTest > testPerformUpdateCheck_emptyLastUpdateCheck FAILED
    value of                   : parse(...)
    expected to be greater than: 2023-04-04T14:14:09.696Z
    but was                    : 2023-04-04T14:14:09.696Z
        at com.google.cloud.tools.jib.plugins.common.UpdateCheckerTest.testPerformUpdateCheck_emptyLastUpdateCheck(UpdateCheckerTest.java:143)

com.google.cloud.tools.jib.plugins.common.UpdateCheckerTest > testPerformUpdateCheck_newVersionFound FAILED
    value of                   : parse(...)
    expected to be greater than: 2023-04-04T14:14:09.854Z
    but was                    : 2023-04-04T14:14:09.854Z
        at com.google.cloud.tools.jib.plugins.common.UpdateCheckerTest.testPerformUpdateCheck_newVersionFound(UpdateCheckerTest.java:83)
suztomo commented 1 year ago

I see the two values are equal. Not greater than. Can you check the assertions? Are they ok to be equal?

emmileaf commented 1 year ago

Looking at UpdateCheckerTest and the corresponding logic in UpdateChecker, I don’t think they should be equal, but also don’t have a windows development setup to easily reproduce this error on. Planning to test out a few initial suspicions in #3979 and see if they lead anywhere:

suztomo commented 1 year ago

I feel that the assertions are written based on an assumption that certain operations take more than 1 millisecond (in the clock). Do you see the same?

emmileaf commented 1 year ago

In case it’s tripping on the difference not being significant, adding a thread.sleep to increase the time gap

I feel that the assertions are written based on an assumption that certain operations take more than 1 millisecond

Ahh turns out this was actually it - thank you @suztomo for the suggestions and for looking into this issue! I was getting thrown off by the change being windows-specific and ended up overthinking here. #3979 should be good to go now :)

suztomo commented 1 year ago

Great. Thank you.