sclorg / container-common-scripts

Apache License 2.0
20 stars 45 forks source link

Move scripts from s2i-core container image to common scripts #210

Closed frenzymadness closed 2 years ago

frenzymadness commented 2 years ago

This change is important for minimal containers because they are not based on s2i-core/s2i-base but still need some of the scripts. This way, we can still maintain them in one place and copy them to whatever image we need.

Related to: https://github.com/sclorg/s2i-python-container/pull/467 Has to be merged together with: https://github.com/sclorg/s2i-base-container/pull/215

frenzymadness commented 2 years ago

I've renamed the folder according to the feedback in the s2i-base-image PR.

phracek commented 2 years ago

What came to my mind. I would say, container-commons-scripts are mainly used for calling building, tagging, generation, and testing.

What about moving it into some repo like s2i-minimal, which will not be a container, or maybe can be, but some group of scripts. Or create in s2i-base-container repository minimal directory with these scripts. Which could be a better solution from my point of view.

frenzymadness commented 2 years ago

What came to my mind. I would say, container-commons-scripts are mainly used for calling building, tagging, generation, and testing.

What about moving it into some repo like s2i-minimal, which will not be a container, or maybe can be, but some group of scripts. Or create in s2i-base-container repository minimal directory with these scripts. Which could be a better solution from my point of view.

And what mechanism would you use for sharing a new repository to other repositories? Do you want to create a separated git submodule for this?

phracek commented 2 years ago

The reason is that scripts like usr/bin/base-usage are now part of downstream repository, but after moving to container-common-scripts the files will disappear. See dist-git repo for s2i-core container s2i-core/tree/root/usr/bin?h=rhscl-3.7-rhel-7

frenzymadness commented 2 years ago

The reason is that scripts like usr/bin/base-usage are now part of downstream repository, but after moving to container-common-scripts the files will disappear. See dist-git repo for s2i-core container s2i-core/tree/root/usr/bin?h=rhscl-3.7-rhel-7

I don't think so. The files will be symlinks which means that you just need to use cp -L when copying them and then they will be at the very same place downstream.

For example, Python container version 3.9 has folder with tests full of symlinks and they are all also in downstream as regular folders.

frenzymadness commented 2 years ago

I'm not saying that this solution is perfect. But I don't see any better and I really do not want to maintain another base/core image for minimal containers.

pkubatrh commented 2 years ago

Our sync tooling should be able to handle symlinks correctly

frenzymadness commented 2 years ago

This is no longer a draft. All feedbacks are incorporated and I believe it's ready to be merged.

phracek commented 2 years ago

[test]

frenzymadness commented 2 years ago

[test] I don't think this PR is the reason for the tests on Fedora to fail.

phracek commented 2 years ago

[test]

frenzymadness commented 2 years ago

There seems to be a problem with pulling ruby image:

Trying to pull repository quay.io/centos7/ruby-24-centos7 ... 
Pulling repository quay.io/centos7/ruby-24-centos7
Error: Status 403 trying to pull repository centos7/ruby-24-centos7: "{\"error\": \"Permission Denied\"}"
Pulling image quay.io/centos7/ruby-24-centos7 failed.
Let's wait 5 seconds and try again.

and it ends with:

image_availability test completed successfully.

This is strange but this does not seem to stop the test. But the log seems to be incomplete so it's hard to find the real reason. @phracek could you please help me here?

phracek commented 2 years ago

There seems to be a problem with pulling ruby image:

Trying to pull repository quay.io/centos7/ruby-24-centos7 ... 
Pulling repository quay.io/centos7/ruby-24-centos7
Error: Status 403 trying to pull repository centos7/ruby-24-centos7: "{\"error\": \"Permission Denied\"}"
Pulling image quay.io/centos7/ruby-24-centos7 failed.
Let's wait 5 seconds and try again.

and it ends with:

image_availability test completed successfully.

This is strange but this does not seem to stop the test. But the log seems to be incomplete so it's hard to find the real reason. @phracek could you please help me here?

This is correct quay.io/centos7/ruby-24-centos7 does not exist. The test suite has to fail. The full log is available here: http://artifacts.dev.testing-farm.io/dd57b00e-f5d4-4d13-b1bf-653ebdb6516b/work-fedoraVZR3Ty/log.txt.gz

The result is here:

13:22:28                 out:  [PASSED] for 'hw' test_incremental_build
13:22:28                 out:  [PASSED] for 'hw' test_build_express_webapp
13:22:28                 out:  [PASSED] for 'clients' express
13:22:28                 out:  [FAILED] for 'clients' pino
13:22:28                 out:  [PASSED] for 'binary' test_run_binary_application

The s2i-nodejs-container failure:

13:20:48                 out: 
13:20:48                 out: npm ERR! code
13:20:48                 out:  ENOENT
13:20:48                 out: npm ERR! syscall spawn git
13:20:48                 out: npm ERR! path git
13:20:48                 out: npm ERR! errno ENOENT
13:20:48                 out: npm ERR! enoent Error while executing:
13:20:49                 out: npm ERR! enoent undefined ls-remote -h -t ssh://git@github.com/isaacs/import-jsx.git
13:20:49                 out: npm ERR! enoent 
13:20:49                 out: npm ERR! enoent 
13:20:49                 out: npm ERR! enoent spawn git ENOENT
13:20:49                 out: npm ERR! enoent This is related to npm not being able to find a file.
13:20:49                 out: npm ERR! enoent 
13:20:49                 out: 
13:20:49                 out: npm ERR! A complete log of this run can be found in:
13:20:49                 out: npm ERR!     /opt/app-root/src/.npm/_logs/2021-11-22T13_20_48_894Z-debug.log
13:20:49                 out: The command '/bin/sh -c /usr/libexec/s2i/assemble' returned a non-zero code: 1
13:20:49                 out: S2I image 'f32/nodejs:12' test FAILED (exit code: 1)
13:20:49                 out: Unable to find image 'f32/nodejs:12-pino' locally
13:20:49                 out: Trying to pull repository docker.io/f32/nodejs ... 
13:20:50                 out: /usr/bin/docker-current: repository docker.io/f32/nodejs not found: does not exist or no pull access.
13:20:50                 out: See '/usr/bin/docker-current run --help'.
13:20:50                 out: S2I image 'f32/nodejs:12' test FAILED (exit code: 125)
13:20:50                 out: S2I image 'f32/nodejs:12' test FAILED (exit code: 125)
13:20:50                 out: 
frenzymadness commented 2 years ago

If the problem is identified to be in the nodejs container and not here, could we merge this?

frenzymadness commented 2 years ago

[test]

frenzymadness commented 2 years ago

All green! I guess we are ready for a merge.

frenzymadness commented 2 years ago

@phracek @pkubatrh wanna do one last check? I think this is ready to be merged and tests agree.

hhorak commented 2 years ago

@frenzymadness I've taken a look and all looks good to me. Merging.