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

Add and upgrade to ruby 3.0 #316

Merged
merged 8 commits into from
Jul 21, 2021
Merged

Add and upgrade to ruby 3.0 #316

merged 8 commits into from
Jul 21, 2021

Conversation

jackorp
Copy link
Contributor

@jackorp jackorp commented Mar 4, 2021

This PR upgrades the s2i to ruby 3.0.

I have not run any tests yet because of missing dependencies.

@centos-ci
Copy link
Collaborator

Can one of the admins verify this patch?

2 similar comments
@centos-ci
Copy link
Collaborator

Can one of the admins verify this patch?

@centos-ci
Copy link
Collaborator

Can one of the admins verify this patch?

@pvalena pvalena self-requested a review March 4, 2021 13:23
@jackorp
Copy link
Contributor Author

jackorp commented Mar 4, 2021

Check failed on local fedora container test with the following outputs:
standard output: https://gist.github.com/jackorp/f32a7135a72211c6954a88584e8ea6db
error output: https://gist.github.com/jackorp/c73c3562483bf485660f0016b3344bac

@pvalena
Copy link
Member

pvalena commented Mar 7, 2021

Blocked by: sclorg/s2i-base-container#203

@pvalena
Copy link
Member

pvalena commented Mar 7, 2021

Check failed on local fedora container test with the following outputs:
standard output: https://gist.github.com/jackorp/f32a7135a72211c6954a88584e8ea6db
error output: https://gist.github.com/jackorp/c73c3562483bf485660f0016b3344bac

better to have them joined, like so:

my command  2>&1

Ex. gist submit, with log file:

my command  2>&1 | tee log_if_you_want.txt | gist -sf "log_file_from_build.txt"

Or if you want to have a terminal copy as well:

my command  2>&1 | tee -a /dev/stderr | gist -sf "log_file_from_build.txt"

From what I've gathered, there's a lot of failed tries to write into /tmp ... is that expected?

And ...

/opt/app-root/src/bundle/ruby/3.0.0/gems/mustermann-1.0.1/lib/mustermann/regular.rb:22:in `initialize': wrong number of arguments (given 2, expected 1) (ArgumentError)
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/mustermann-1.0.1/lib/mustermann/pattern.rb:59:in `new'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/mustermann-1.0.1/lib/mustermann/pattern.rb:59:in `block in new'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/mustermann-1.0.1/lib/mustermann/equality_map.rb:39:in `fetch'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/mustermann-1.0.1/lib/mustermann/pattern.rb:59:in `new'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/mustermann-1.0.1/lib/mustermann.rb:67:in `new'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/mustermann-1.0.1/lib/mustermann.rb:70:in `block in new'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/mustermann-1.0.1/lib/mustermann.rb:70:in `map'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/mustermann-1.0.1/lib/mustermann.rb:70:in `new'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra/base.rb:1638:in `compile'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra/base.rb:1626:in `compile!'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra/base.rb:1268:in `error'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra/base.rb:1836:in `<class:Base>'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra/base.rb:894:in `<module:Sinatra>'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra/base.rb:22:in `<top (required)>'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra/main.rb:1:in `require'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra/main.rb:1:in `<top (required)>'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra.rb:1:in `require'
	from /opt/app-root/src/bundle/ruby/3.0.0/gems/sinatra-2.0.0/lib/sinatra.rb:1:in `<top (required)>'
	from /opt/app-root/src/app.rb:1:in `require'
	from /opt/app-root/src/app.rb:1:in `<top (required)>'

Could you create a reproducer for that? / And file it upstream / into Fedora etc. (where suitable).

@pvalena
Copy link
Member

pvalena commented Mar 7, 2021

In my attempt, I've also noticed possible issue- the tests try to use already existing Fedora container- could you investigate?

https://git.io/Jqk7h
(Either that, or the :0-testapp has failed to build, and the error is missing.)

EDIT: yes, it seems the app fails to build using Dockerfile, because it cannot access the /tmp. this renders the test suite unusable, though (I've created a PR for that).

The error output is the same though: https://git.io/JqkF5

pvalena added a commit that referenced this pull request Mar 7, 2021
to make the test suite more user-friendly.

Related: PR #316.
pvalena added a commit that referenced this pull request Mar 19, 2021
to make the test suite more user-friendly.

Related: PR #316.
@jackorp
Copy link
Contributor Author

jackorp commented Mar 23, 2021

/tmp not writeable reported here as that is an issue of underlying images.

@pvalena
Copy link
Member

pvalena commented Mar 23, 2021

@jackorp can the TMP dir be created locally, in the meantime? IOW in the app-dir.

@jackorp
Copy link
Contributor Author

jackorp commented Mar 23, 2021

@jackorp can the TMP dir be created locally, in the meantime? IOW in the app-dir.

I am not sure what do you want to achieve. Do you mean to basically override the bundler option?

@pvalena
Copy link
Member

pvalena commented Mar 23, 2021

@jackorp can the TMP dir be created locally, in the meantime? IOW in the app-dir.

I am not sure what do you want to achieve. Do you mean to basically override the bundler option?

Yes. I think it's --path /some/path ... or is the tmp used for something else?

@jackorp
Copy link
Contributor Author

jackorp commented Mar 23, 2021

@jackorp can the TMP dir be created locally, in the meantime? IOW in the app-dir.

I am not sure what do you want to achieve. Do you mean to basically override the bundler option?

Yes. I think it's --path /some/path ... or is the tmp used for something else?

Hmm, it seems like that won't be of help. My wild guess is that bundler needs a temporary directory when installing (for whatever reason) and the --path switch just sets where the final install path is.

@pvalena
Copy link
Member

pvalena commented Mar 24, 2021

@jackorp can the TMP dir be created locally, in the meantime? IOW in the app-dir.

I am not sure what do you want to achieve. Do you mean to basically override the bundler option?

Yes. I think it's --path /some/path ... or is the tmp used for something else?

Hmm, it seems like that won't be of help. My wild guess is that bundler needs a temporary directory when installing (for whatever reason) and the --path switch just sets where the final install path is.

Have you tried? Maybe this should be an upstream issue.

@jackorp
Copy link
Contributor Author

jackorp commented Mar 24, 2021

@jackorp can the TMP dir be created locally, in the meantime? IOW in the app-dir.

I am not sure what do you want to achieve. Do you mean to basically override the bundler option?

Yes. I think it's --path /some/path ... or is the tmp used for something else?

Hmm, it seems like that won't be of help. My wild guess is that bundler needs a temporary directory when installing (for whatever reason), and the --path switch just sets where the final install path is.

Have you tried? Maybe this should be an upstream issue.

I did try, but in the end, it looks like it does not matter a whole lot for us.

More tests ran than before, and even though /tmp is not writeable it installed the gems and did some tests successfully: see log

There are messages about /tmp not writeable (that error is from Ruby's tmpdir library) on this line. However, the gems get installed which I think is what matters for us.

@pvalena
Copy link
Member

pvalena commented Mar 24, 2021

I did try, but in the end, it looks like it does not matter a whole lot for us.

More tests ran than before, and even though /tmp is not writeable it installed the gems and did some tests successfully: see log

There are messages about /tmp not writeable (that error is from Ruby's tmpdir library) on this line. However, the gems get installed which I think is what matters for us.

Sorry, I got the impression that these are bundler errors which cause some of the tests to fail. Instead it's just ruby churn.

So it could be switched off by something like export RUBYOPT=-W0, right? Thanks for investigation!

(FTR: The mustermann issue is blocked on me.)

@jackorp
Copy link
Contributor Author

jackorp commented Mar 24, 2021

> Sorry, I got the impression that these are bundler errors which cause some of the tests to fail. Instead it's just ruby churn.

I had the same impression, however, on closer inspection, I found out that some bundle installs failed because of minitest.

So it could be switched off by something like export RUBYOPT=-W0, right? Thanks for investigation!

Should work for silencing it, I'll look into it later (if we want to anyway).

And with that, I'd like to point out that a test throws an exception with unwritable /tmp, like this rack error. But there should be a way to tell it where to store the file instead of /tmp.

@pvalena
Copy link
Member

pvalena commented Mar 25, 2021

Sorry, I got the impression that these are bundler errors which cause some of the tests to fail. Instead it's just ruby churn.

I had the same impression, however, on closer inspection, I found out that some bundle installs failed because of minitest.

Now this is interesting. Coud you look into that? The app'll probabrly need a sample app dependency refresh.

@jackorp
Copy link
Contributor Author

jackorp commented Mar 25, 2021

Sorry, I got the impression that these are bundler errors which cause some of the tests to fail. Instead it's just ruby churn.

I had the same impression, however, on closer inspection, I found out that some bundle installs failed because of minitest.

Now this is interesting. Coud you look into that? The app'll probabrly need a sample app dependency refresh.

I'll look into it.

@jackorp
Copy link
Contributor Author

jackorp commented Mar 26, 2021

So I did a little dependency refresh on the example rails app for Ruby 3.0 and hit a new error with bundle exec "rake assets:precompile" which is related to ruby 3.0: see log. It seems to be one of the rails' components, so we probably will need to refresh the example app in the future.

Other than that, the /tmp not writeable (which causes rack fails currently), and mustermann there seem to be no other problems.

@jackorp jackorp force-pushed the s2i-ruby_3.0 branch 2 times, most recently from 6200233 to e4f4f15 Compare April 26, 2021 12:29
@jackorp jackorp changed the title WIP: Add and upgrade to ruby 3.0 Add and upgrade to ruby 3.0 Apr 26, 2021
@pvalena
Copy link
Member

pvalena commented Apr 30, 2021

Not sure where the culprit lies (maybe you need to rebase?), this is the diff I've landed on while applying your PR:

https://git.io/J3sCK

@jackorp
Copy link
Contributor Author

jackorp commented Apr 30, 2021

Not sure where the culprit lies (maybe you need to rebase?), this is the diff I've landed on while applying your PR:

https://git.io/J3sCK

yeah there were some things that went wrong when rebasing on newest master apparently... I'll fix that.

@phracek
Copy link
Member

phracek commented Jun 10, 2021

s2i-base and s2i-core were re-built. Let's re-run tests.

[test]

@phracek
Copy link
Member

phracek commented Jun 10, 2021

@jackorp
The Fedora and RHEL7 test failed on:

08:31:23 CMD exec bundle exec "rackup -P /tmp/rack.pid --host 0.0.0.0 --port 8080"
08:31:23 + readarray -d @ -t git_url_parts
08:31:23 /root/sources/3.0/test/test-lib.sh: line 870: readarray: -d: invalid option
08:31:23 readarray: usage: readarray [-n count] [-O origin] [-s count] [-t] [-u fd] [-C callback] [-c quantum] [array]

This was replaced by code:
Please rebase container-common-scripts

RHEL8 test failed on network issues:

        STEP 3: RUN bundle install --path ./bundle
        Fetching source index from https://rubygems.org/
        Network error while fetching
        https://rubygems.org/quick/Marshal.4.8/nokogiri-1.11.3-x86_64-linux.gemspec.rz
        (execution expired)
        Error: error building at STEP "RUN bundle install --path ./bundle": error while running runtime: exit status 17
        ERROR: The image cannot be built from /root/sources/examples/from-dockerfile/Dockerfile and application https://github.com/sclorg/rails-ex.git.
        Terminating the Dockerfile build.

@pvalena
Copy link
Member

pvalena commented Jun 10, 2021

@jackorp
The Fedora and RHEL7 test failed on:

08:31:23 CMD exec bundle exec "rackup -P /tmp/rack.pid --host 0.0.0.0 --port 8080"
08:31:23 + readarray -d @ -t git_url_parts
08:31:23 /root/sources/3.0/test/test-lib.sh: line 870: readarray: -d: invalid option
08:31:23 readarray: usage: readarray [-n count] [-O origin] [-s count] [-t] [-u fd] [-C callback] [-c quantum] [array]

This was replaced by code:
Please rebase container-common-scripts

If tests are broken, as of now, it is not up to this PR to fix them. I think that change may even render it unable to auto-rebase. It container-common-scripts should be updated outside of this PR.

RHEL8 test failed on network issues:

        STEP 3: RUN bundle install --path ./bundle
        Fetching source index from https://rubygems.org/
        Network error while fetching
        https://rubygems.org/quick/Marshal.4.8/nokogiri-1.11.3-x86_64-linux.gemspec.rz
        (execution expired)
        Error: error building at STEP "RUN bundle install --path ./bundle": error while running runtime: exit status 17
        ERROR: The image cannot be built from /root/sources/examples/from-dockerfile/Dockerfile and application https://github.com/sclorg/rails-ex.git.
        Terminating the Dockerfile build.

Is that random / temporary or something that needs to be fixed in the infrastructure first?

@pvalena
Copy link
Member

pvalena commented Jun 10, 2021

Oh I think the problem is that instead of the latest common commit in this repository (which is already up-to-date), it uses the one from the PR. It does not auto-rebase the common for running the tests... that's unfortunate.

Workaround: @jackorp maybe there's a rebase button?

Solution @phracek maybe that could be enhanced? Instead of running the common commit in the PR, we could either:

  • Checkout common latest master commit; or
  • Checkout the commit from the latest commit of this repository, e.g. $ git checkout origin/master common

@phracek
Copy link
Member

phracek commented Jun 11, 2021

@pvalena @jackorp Sorry I have meant common directory. Checkout to the latest commit. :)

@jackorp
Copy link
Contributor Author

jackorp commented Jun 11, 2021

I have rebased the PR branch.

@phracek
Copy link
Member

phracek commented Jun 11, 2021

[test]

@pvalena
Copy link
Member

pvalena commented Jun 11, 2021

@pvalena @jackorp Sorry I have meant common directory. Checkout to the latest commit. :)

Yes, I know, that was apparent from your initial text. my comment was relevant to that.

Please give it a go, so we can have a permanent solution, avoiding future PR rebases, due compatible upstream changes, just for testing purposes....

@phracek
Copy link
Member

phracek commented Jun 14, 2021

Please give it a go, so we can have a permanent solution, avoiding future PR rebases, due compatible upstream changes, just for testing purposes....

The standard SCLorg solution is simple. We do not want CI to clone container-common-scripts and move to the latest commit.
Updating container-common-scripts as a part of PR is the best solution for now. That's how it is going for a long time.
Each SCLorg repository has the stable container-common-scripts "version". In case that there is a bug in container-common-scripts then the SCLorg repositories are not affected :)
For know you have to rebase directory common.

But in the meantime, I am working on container-ci-suite (https://github.com/phracek/container-ci-suite) which will be available on PyPi so container-common-scripts will be replaced. But it is a far future.

@pvalena
Copy link
Member

pvalena commented Jun 15, 2021

The standard SCLorg solution is simple. We do not want CI to clone container-common-scripts and move to the latest commit.
Updating container-common-scripts as a part of PR is the best solution for now. That's how it is going for a long time.
Each SCLorg repository has the stable container-common-scripts "version". In case that there is a bug in container-common-scripts then the SCLorg repositories are not affected :)
For know you have to rebase directory common.

The whole argument is moot, as I've realized that the problem is more serious, and does not affect only the common scripts, and it is a bug. (And it definitely is not "best".)

I'll file an issue for that.

But in the meantime, I am working on container-ci-suite (https://github.com/phracek/container-ci-suite) which will be available on PyPi so container-common-scripts will be replaced. But it is a far future.

Great!

@pvalena
Copy link
Member

pvalena commented Jun 28, 2021

Let's get this merged and we can fix the remaining tests later on (sclorg/rails-ex#134).

@phracek
Copy link
Member

phracek commented Jul 12, 2021

[test]

@phracek
Copy link
Member

phracek commented Jul 19, 2021

Let's get this merged and we can fix the remaining tests later on (sclorg/rails-ex#134).

Hi @pvalena , and what about the problem with network issues in RHEL8?

@phracek
Copy link
Member

phracek commented Jul 19, 2021

---> Running 'bundle install --retry 2 --deployment --without test' ...
Fetching gem metadata from https://rubygems.org/.
Retrying dependency api due to error (2/3): Bundler::HTTPError Network error while fetching https://index.rubygems.org/api/v1/dependencies?gems=actioncable%2Cactionmailer%2Cactionpack%2Cactionview%2Cactivejob%2Cactivemodel%2Cactiverecord%2Cactivesupport%2Caddressable%2Carel%2Cbindex%2Cbuilder%2Cbyebug%2Ccapybara%2Cchildprocess%2Ccoffee-rails%2Ccoffee-script%2Ccoffee-script-source%2Cconcurrent-ruby%2Ccrass%2Cerubi%2Cexecjs%2Cffi%2Cglobalid%2Ci18n%2Cjbuilder%2Cloofah%2Cmail%2Cmethod_source%2Cmini_mime%2Cmini_portile2%2Cminitest%2Cnio4r%2Cnokogiri%2Cpublic_suffix%2Cracc%2Crack%2Crack-test%2Crails-dom-testing%2Crails-html-sanitizer%2Crailties%2Crake%2Crb-fsevent%2Crb-inotify%2Cthor%2Cthread_safe%2Ctzinfo%2Cwebsocket-driver%2Cwebsocket-extensions%2Cxpath (execution expired)
Retrying dependency api due to error (3/3): Bundler::HTTPError Network error while fetching https://index.rubygems.org/api/v1/dependencies?gems=actioncable%2Cactionmailer%2Cactionpack%2Cactionview%2Cactivejob%2Cactivemodel%2Cactiverecord%2Cactivesupport%2Caddressable%2Carel%2Cbindex%2Cbuilder%2Cbyebug%2Ccapybara%2Cchildprocess%2Ccoffee-rails%2Ccoffee-script%2Ccoffee-script-source%2Cconcurrent-ruby%2Ccrass%2Cerubi%2Cexecjs%2Cffi%2Cglobalid%2Ci18n%2Cjbuilder%2Cloofah%2Cmail%2Cmethod_source%2Cmini_mime%2Cmini_portile2%2Cminitest%2Cnio4r%2Cnokogiri%2Cpublic_suffix%2Cracc%2Crack%2Crack-test%2Crails-dom-testing%2Crails-html-sanitizer%2Crailties%2Crake%2Crb-fsevent%2Crb-inotify%2Cthor%2Cthread_safe%2Ctzinfo%2Cwebsocket-driver%2Cwebsocket-extensions%2Cxpath (execution expired)

@pvalena
Copy link
Member

pvalena commented Jul 19, 2021

---> Running 'bundle install --retry 2 --deployment --without test' ...
Fetching gem metadata from https://rubygems.org/.
Retrying dependency api due to error (2/3): Bundler::HTTPError Network error while fetching https://index.rubygems.org/api/v1/dependencies?gems=actioncable%2Cactionmailer%2Cactionpack%2Cactionview%2Cactivejob%2Cactivemodel%2Cactiverecord%2Cactivesupport%2Caddressable%2Carel%2Cbindex%2Cbuilder%2Cbyebug%2Ccapybara%2Cchildprocess%2Ccoffee-rails%2Ccoffee-script%2Ccoffee-script-source%2Cconcurrent-ruby%2Ccrass%2Cerubi%2Cexecjs%2Cffi%2Cglobalid%2Ci18n%2Cjbuilder%2Cloofah%2Cmail%2Cmethod_source%2Cmini_mime%2Cmini_portile2%2Cminitest%2Cnio4r%2Cnokogiri%2Cpublic_suffix%2Cracc%2Crack%2Crack-test%2Crails-dom-testing%2Crails-html-sanitizer%2Crailties%2Crake%2Crb-fsevent%2Crb-inotify%2Cthor%2Cthread_safe%2Ctzinfo%2Cwebsocket-driver%2Cwebsocket-extensions%2Cxpath (execution expired)
Retrying dependency api due to error (3/3): Bundler::HTTPError Network error while fetching https://index.rubygems.org/api/v1

These are simple timeouts. It's only related to the infrastructure it's running on. Nothing to do from our side. I'll be glad to debug if you send me some more logs (--debug output), or an ssh connection, if you can reproduce it somewhere else.

Feel free to rerun the tests several times to be sure.

@pvalena
Copy link
Member

pvalena commented Jul 19, 2021

@pvalena
Copy link
Member

pvalena commented Jul 19, 2021

Please note that Dockerfiles in this PR are either way used in production right now (having them merged or not does not change that). What's more, the longer this PR needlessly hangs (it's not broken!), the more changes are made to the other Dockerfiles which this PR will need to add. It's like maintaining a fork, consuming time and it brings no benefit to this project not to have these new Dockerfiles (i.e. this PR does not break anything).

@phracek
Copy link
Member

phracek commented Jul 20, 2021

[test]

@phracek phracek merged commit 179eb26 into sclorg:master Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants