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

Kokoro Windows continuous job failing after image migration #3978

Closed
mpeddada1 opened this issue Apr 3, 2023 · 7 comments · Fixed by #3979
Closed

Kokoro Windows continuous job failing after image migration #3978

mpeddada1 opened this issue Apr 3, 2023 · 7 comments · Fixed by #3979

Comments

@mpeddada1
Copy link
Contributor

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
Copy link
Contributor

emmileaf commented Apr 3, 2023

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

ImmutableMap.of("replace" + System.lineSeparator() + "this", "creationTime: 1234"));

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

new StringSubstitutorReader(
Files.newBufferedReader(buildFilePath, Charsets.UTF_8), templater)) {

@emmileaf
Copy link
Contributor

emmileaf commented Apr 4, 2023

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
Copy link
Contributor

suztomo commented Apr 5, 2023

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

@emmileaf
Copy link
Contributor

emmileaf commented Apr 6, 2023

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:

  • In case it’s tripping on the difference not being significant, adding a thread.sleep to increase the time gap
  • There are a lot of File read/write interactions in the related logic that specify StandardCharsets.UTF_8, and I wonder if something is missing or incompatible here with the new windows environment default of ​​Windows-1252 - hopefully printing out the lastUpdateCheck File contents in more places can give more insights in addition to this error message.

@suztomo
Copy link
Contributor

suztomo commented Apr 6, 2023

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
Copy link
Contributor

emmileaf commented Apr 6, 2023

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
Copy link
Contributor

suztomo commented Apr 6, 2023

Great. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants