redhat-developer / s2i-dotnetcore

.NET Core OpenShift images
Apache License 2.0
112 stars 192 forks source link

Verify package list in x86_64 containers only #446

Closed omajid closed 1 year ago

omajid commented 1 year ago

The packages in containers for each architecture are different. Sometimes that's because of transitive dependencies, such as lttng-ust not needed for Mono-builds of .NET. Sometimes there are other reasons. Handle that by using architecture-specific package lists.

We can generate the package list on an x86_64 box using:

for arch in aarch64 ppc64le s390x x86_64; do
    podman run -it --arch $arch $container_name $commands > $file.$arch.list
done
omajid commented 1 year ago

While testing this, I see that https://github.com/redhat-developer/s2i-dotnetcore/tree/master/6.0/build/test/newtemplate fails on aarch64/arm64 with an error like this:

Microsoft (R) Build Engine version 17.0.1+b177f8fa7 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  src -> /opt/app-root/src/bin/Release/net6.0/src.dll
/usr/bin/sh : warning : setlocale: LC_ALL: cannot change locale (en_US.UTF-8) [/opt/app-root/src/src.csproj]
/usr/bin/sh : warning : setlocale: LC_ALL: cannot change locale (en_US.UTF-8) [/opt/app-root/src/src.csproj]
  npm WARN read-shrinkwrap This version of npm is compatible with lockfileVersion@1, but package-lock.json was generated for lockfileVersion@2. I'\''ll try to do my best with it!

  > nice-napi@1.0.2 install /opt/app-root/src/ClientApp/node_modules/nice-napi
  > node-gyp-build

sh : warning : setlocale: LC_ALL: cannot change locale (en_US.UTF-8) [/opt/app-root/src/src.csproj]
  gyp ERR! find Python 
  gyp ERR! find Python checking Python explicitly set from command line or npm configuration
  gyp ERR! find Python - "--python=" or "npm config get python" is "/usr/bin/python3"
  gyp ERR! find Python - "/usr/bin/python3" is not in PATH or produced an error
  gyp ERR! find Python Python is not set from environment variable PYTHON
  gyp ERR! find Python checking if "python3" can be used
  gyp ERR! find Python - "python3" is not in PATH or produced an error
  gyp ERR! find Python checking if "python" can be used
  gyp ERR! find Python - "python" is not in PATH or produced an error
  gyp ERR! find Python 
  gyp ERR! find Python **********************************************************
  gyp ERR! find Python You need to install the latest version of Python.
  gyp ERR! find Python Node-gyp should be able to find and use Python. If not,
  gyp ERR! find Python you can try one of the following options:
  gyp ERR! find Python - Use the switch --python="/path/to/pythonexecutable"
  gyp ERR! find Python   (accepted by both node-gyp and npm)
  gyp ERR! find Python - Set the environment variable PYTHON
  gyp ERR! find Python - Set the npm configuration variable python:
  gyp ERR! find Python   npm config set python "/path/to/pythonexecutable"
  gyp ERR! find Python For more information consult the documentation at: 
  gyp ERR! find Python https://github.com/nodejs/node-gyp#installation
  gyp ERR! find Python **********************************************************
  gyp ERR! find Python 
  gyp ERR! configure error 
EXEC : gyp ERR! stack error : Could not find any Python installation to use [/opt/app-root/src/src.csproj]
  gyp ERR! stack     at PythonFinder.fail (/opt/app-root/src/ClientApp/node_modules/node-gyp/lib/find-python.js:330:47)
  gyp ERR! stack     at PythonFinder.runChecks (/opt/app-root/src/ClientApp/node_modules/node-gyp/lib/find-python.js:159:21)
  gyp ERR! stack     at PythonFinder.<anonymous> (/opt/app-root/src/ClientApp/node_modules/node-gyp/lib/find-python.js:202:16)
  gyp ERR! stack     at PythonFinder.execFileCallback (/opt/app-root/src/ClientApp/node_modules/node-gyp/lib/find-python.js:294:16)
  gyp ERR! stack     at exithandler (child_process.js:390:5)
  gyp ERR! stack     at ChildProcess.errorhandler (child_process.js:402:5)
  gyp ERR! stack     at ChildProcess.emit (events.js:400:28)
  gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:283:12)
  gyp ERR! stack     at onErrorNT (internal/child_process.js:472:16)
  gyp ERR! stack     at processTicksAndRejections (internal/process/task_queues.js:82:21)
  gyp ERR! System Linux 6.0.5-200.fc36.aarch64
  gyp ERR! command "/usr/bin/node" "/opt/app-root/src/ClientApp/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
  gyp ERR! cwd /opt/app-root/src/ClientApp/node_modules/nice-napi
  gyp ERR! node -v v14.21.1
  gyp ERR! node-gyp -v v8.4.1
  gyp ERR! not ok  

  > core-js@3.23.2 postinstall /opt/app-root/src/ClientApp/node_modules/core-js
  > node -e "try{require('\''./postinstall'\'')}catch(e){}"
...
...
...
/opt/app-root/src/src.csproj(36,5): error MSB3073: The command "npm install" exited with code -1.
Error: error building at STEP "RUN /tmp/scripts/assemble": error while running runtime: exit status 1'

@aslicerh have you seen this before?

aslicerh commented 1 year ago

@omajid We've actually talked about this before. It was an issue with how npm handles dependencies, where this maybe should be an optional dependency.

Here's the issue that @tmds pointed to about it: https://github.com/angular/angular-cli/issues/21547

The issue was architecture specific, and the solution ended up being to move the at least nodejs 16, which fixes it. (which is why .NET 7 doesn't have this problem).

tmds commented 1 year ago

These list of packages need to manually be maintained, and, with this, we need several architectures to verify, and maintain them.

Our images install the .NET bits needed, and then some extra packages, and we document why those packages are there:

https://github.com/redhat-developer/s2i-dotnetcore/blob/47280d183b7b6a45e4713f4240c4ca739ddb2179/7.0/runtime/Dockerfile.rhel8#L49-L60

We wouldn't remove any of these as otherwise we break what is listed there.

We have functional tests that verify what we promise to be working.

The goal of this test is not to guarantee that we preserve all packages when moving from one base to another. It is to make us look at the change, and see if we haven't accidentally removed something 'essential'. Preferably, such things would already be covered by functional tests.

Our Dockerfile is the same for all architectures.

For the intended purpuse, maybe we can run the test and maintain the list for x64 only?

tmds commented 1 year ago

We've actually talked about this before. It was an issue with how npm handles dependencies, where this maybe should be an optional dependency.

I remember chatting about this for .NET 7. I didn't know the issue also occurs with .NET 6 on arm64.

I don't know if we want to address this at this point. We should at least include it with the known issues, and skip the test on arm64 if it is expected to fail.

tmds commented 1 year ago

I prefer functional tests where a fail means an issue, over tests like this where a fail means you need to manually inspect and acknowledge things are still ok.

The manual work increases further with this PR.

If you want to do this on multiple architectures, can you add a script that does: _We can generate the package list on an x8664 box using:. I think it involves more work than the phrase suggests.

tmds commented 1 year ago

At I suggested in my first comment, I think we should consider to run this on x64 only.

I have no other feedback.

omajid commented 1 year ago

Okay, I changed it to run on x86_64 only.

tmds commented 1 year ago

Thanks @omajid!

omajid commented 1 year ago

@aslicerh if this looks okay to you, can you please merge this?

aslicerh commented 1 year ago

@omajid Done!

omajid commented 1 year ago

Thanks!