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

Run bundle with systemd group #62

Merged
merged 1 commit into from
Jun 11, 2020
Merged

Conversation

simaishi
Copy link
Contributor

@simaishi simaishi commented Jun 5, 2020

This was added to appliance build in ManageIQ/manageiq-appliance-build#396, but got missed when switched to rpm based. Sorry @agrare

Also changed to use "bundle config set" to set with/without options as passing those flags
to bundle install is deprecated.

Also changed to use "config set" to set with/without options as passing flags
to bundle install is deprecated.
@simaishi
Copy link
Contributor Author

simaishi commented Jun 5, 2020

Oh wait... would this become a problem in podified build?

@simaishi
Copy link
Contributor Author

simaishi commented Jun 5, 2020

^ @carbonin

@agrare
Copy link
Member

agrare commented Jun 5, 2020

@simaishi we have a gate on the #supports_systemd? call which checks to ensure this is an appliance first: https://github.com/ManageIQ/manageiq/blob/af6c7830597b521c865608d3555af2a5b7b3159c/lib/miq_environment.rb#L17-L20

Even if that didn't work for some reason we catch the load error here so if it wasn't in the bundle it'll fallback to the normal mode of starting workers

@carbonin
Copy link
Member

carbonin commented Jun 5, 2020

I'm not sure. Does the rpm actually require systemd or any of it's libs be installed?

If we don't require/use the gem in pods I would guess it would be okay.

@chessbyte chessbyte added the bug Something isn't working label Jun 11, 2020
@@ -61,7 +61,9 @@ def populate_gem_home

shell_cmd("gem env")
shell_cmd("gem install mime-types -v 2.6.1")
shell_cmd("bundle _#{bundler_version}_ install --with qpid_proton --without test:development:metric_fu --jobs #{cpus} --retry 3")
shell_cmd("bundle config set --local with qpid_proton systemd")
shell_cmd("bundle config set --local without 'test:development:metric_fu'")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, metric_fu has been gone a long time and I think we can drop this now (followup PR)

@Fryguy Fryguy merged commit 9ebebe3 into ManageIQ:master Jun 11, 2020
@simaishi simaishi deleted the systemd_group branch June 11, 2020 18:58
@carbonin
Copy link
Member

So this broke pods:

{"@timestamp":"2020-06-15T19:52:15.811827 ","hostname":"orchestrator-995fd99bc-8p8tb","pid":8,"tid":"2acc9c5cf95c","level":"err","message":"MIQ(MiqServer#sync_workers) Failed to sync_workers for class: MiqCockpitWsWorker"}
{"@timestamp":"2020-06-15T19:52:15.812116 ","hostname":"orchestrator-995fd99bc-8p8tb","pid":8,"tid":"2acc9c5cf95c","level":"err","message":"[Errno::EACCES]: Permission denied @ rb_sysopen - /etc/systemd/system/cockpit_ws.target  Method:[block (2 levels) in \u003cclass:LogProxy\u003e]"}
{"@timestamp":"2020-06-15T19:52:15.812280 ","hostname":"orchestrator-995fd99bc-8p8tb","pid":8,"tid":"2acc9c5cf95c","level":"err","message":"/var/www/miq/vmdb/app/models/miq_worker/systemd_common.rb:15:in `write'\n/var/www/miq/vmdb/app/models/miq_worker/systemd_common.rb:15:in `write'\n/var/www/miq/vmdb/app/models/miq_worker/systemd_common.rb:15:in `ensure_systemd_files'\n/var/www/miq/vmdb/app/models/miq_server/worker_management/monitor.rb:54:in `block in sync_workers'\n/var/www/miq/vmdb/app/models/miq_server/worker_management/monitor.rb:49:in `each'\n/var/www/miq/vmdb/app/models/miq_server/worker_management/monitor.rb:49:in `sync_workers'\n/var/www/miq/vmdb/lib/workers/evm_server.rb:254:in `start_workers'\n/var/www/miq/vmdb/lib/workers/evm_server.rb:130:in `start_server'\n/var/www/miq/vmdb/lib/workers/evm_server.rb:42:in `block in start_servers'\n/var/www/miq/vmdb/lib/workers/evm_server.rb:273:in `block in as_each_server'\n/var/www/miq/vmdb/lib/workers/evm_server.rb:271:in `each'\n/var/www/miq/vmdb/lib/workers/evm_server.rb:271:in `as_each_server'\n/var/www/miq/vmdb/lib/workers/evm_server.rb:42:in `start_servers'\n/var/www/miq/vmdb/lib/workers/evm_server.rb:28:in `start'\n/var/www/miq/vmdb/lib/workers/evm_server.rb:81:in `start'\n/var/www/miq/vmdb/lib/workers/bin/evm_server.rb:4:in `\u003cmain\u003e'"}
irb(main):001:0> MiqGenericWorker.systemd_worker?
=> true
irb(main):002:0> MiqEnvironment::Command.supports_systemd?
=> true

We can probably come up with something to solve this in the source if it isn't easy to back this out just for pods.

@Fryguy
Copy link
Member

Fryguy commented Jun 15, 2020

This could be my fault because I changed the way appliances are detected here: ManageIQ/manageiq@40620c9#diff-bd3bf5ebb076d419f16ea1eefa515cc9

Is the env var APPLIANCE set in containers?

@carbonin
Copy link
Member

carbonin commented Jun 15, 2020

Is the env var APPLIANCE set in containers?

Yes, because we source /etc/default/evm

@agrare
Copy link
Member

agrare commented Jun 15, 2020

I'll add an explicit !is_container to supports_systemd?

@Fryguy
Copy link
Member

Fryguy commented Jun 15, 2020

@agrare I think I'd rather just fix is_container and is_appliance methods since they are wrong at the moment.

EDIT: Actually it can't hurt to do both since we are sure we can't do systemd in containers.

@carbonin
Copy link
Member

Yeah, was about to do that @agrare, just trying to figure out how to rebuild pods images using rpms with my manageiq branch to test.

@simaishi
Copy link
Contributor Author

simaishi commented Jun 15, 2020

When we say "appliance" (for env var and is_appliance), that should only include VM appliances? If so, having APPLIANCE env var set in pods is wrong...

@Fryguy
Copy link
Member

Fryguy commented Jun 15, 2020

@simaishi IMO, yes. We had discussed coming up with a different term for a "build" vs a literal "appliance": ManageIQ/manageiq#20107 (comment)

@Fryguy
Copy link
Member

Fryguy commented Jun 15, 2020

However, for now, I'm 👍 with @agrare's idea to just short circuit supports_systemd? directly.

@carbonin
Copy link
Member

Building this patch into container images now ...

16:03:42:~/Source/manageiq (no_systemd_in_containers)$ git diff upstream/master
diff --git a/lib/miq_environment.rb b/lib/miq_environment.rb
index 97813acb6a..c397a97ece 100644
--- a/lib/miq_environment.rb
+++ b/lib/miq_environment.rb
@@ -16,7 +16,7 @@ module MiqEnvironment
 
     def self.supports_systemd?
       return @supports_systemd unless @supports_systemd.nil?
-      @supports_systemd = is_appliance? && supports_command?('systemctl')
+      @supports_systemd = !is_container && is_appliance? && supports_command?('systemctl')
     end
 
     def self.supports_nohup_and_backgrounding?

@carbonin
Copy link
Member

Well that didn't work. HALP

I'm running this in pods root dir:
RPM_BUILD_OPTIONS=images/manageiq-base/options bin/build -b -d images/ -npr docker.io/carbonin

With this file:

16:20:51:~/Source/manageiq-pods (master *)$ cat images/manageiq-base/options/options.yml 
---
repos:
  ref:             master
  manageiq:
    url: https://github.com/carbonin/manageiq.git
    ref: no_systemd_in_containers

Should that have pulled my manageiq ref here?

 ---> ManageIQ::RPMBuild::SetupSourceRepos#initialize

 ---> ManageIQ::RPMBuild::SetupSourceRepos#populate

 ---> ManageIQ::RPMBuild::SetupSourceRepos#clean_build_dir

 ---> ManageIQ::RPMBuild::SetupSourceRepos#setup_rpm_spec_repo

 ---> ManageIQ::RPMBuild::SetupSourceRepos#setup_source_repo

 ---> git clone --depth 1 -b master https://github.com/ManageIQ/manageiq-appliance.git manageiq-appliance
...

 ---> git clone --depth 1 -b master https://github.com/ManageIQ/manageiq.git manageiq
Cloning into 'manageiq'...

@simaishi
Copy link
Contributor Author

Tried running RPM_BUILD_OPTIONS=images/manageiq-base/options bin/build -b -d images -r simaishi -n with your options.yml

$ cat images/manageiq-base/options/options.yml
repos:
  ref:             master
  manageiq:
    url: https://github.com/carbonin/manageiq.git
    ref: no_systemd_in_containers

And it pulled the right branch. Not sure what the difference is...

 ---> ManageIQ::RPMBuild::SetupSourceRepos#initialize

 ---> ManageIQ::RPMBuild::SetupSourceRepos#populate

 ---> ManageIQ::RPMBuild::SetupSourceRepos#clean_build_dir

 ---> ManageIQ::RPMBuild::SetupSourceRepos#setup_rpm_spec_repo

 ---> ManageIQ::RPMBuild::SetupSourceRepos#setup_source_repo

 ---> git clone --depth 1 -b master https://github.com/ManageIQ/manageiq-appliance.git manageiq-appliance
Cloning into 'manageiq-appliance'...
remote: Enumerating objects: 117, done.
remote: Counting objects: 100% (117/117), done.
remote: Compressing objects: 100% (92/92), done.
remote: Total 117 (delta 3), reused 73 (delta 1), pack-reused 0
Receiving objects: 100% (117/117), 38.58 KiB | 3.86 MiB/s, done.
Resolving deltas: 100% (3/3), done.

 ---> git clone --depth 1 -b no_systemd_in_containers https://github.com/carbonin/manageiq.git manageiq

@carbonin
Copy link
Member

Ugh, well I'm using podman because something with docker build is broken.

But it probably has something to do with this:

16:35:39:~/Source/manageiq-pods (master *)$ podman run -it --rm -v /home/ncarboni/Source/manageiq-pods/images//manageiq-base/rpms:/root/BUILD/rpms -v /home/ncarboni/Source/manageiq-pods/images/manageiq-base/options:/root/OPTIONS manageiq/rpm_build:latest
Don't run Bundler as root. Bundler can ask for sudo if it is needed, and installing your bundle as root will break this application for all non-root users on this machine.
Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
...
Bundle complete! 6 Gemfile dependencies, 27 gems now installed.
...
[root@d59b6f31e1f7 build_scripts]# ls /root/OPTIONS/
ls: cannot open directory '/root/OPTIONS/': Permission denied

😫

simaishi pushed a commit that referenced this pull request Jun 18, 2020
Run bundle with systemd group

(cherry picked from commit 9ebebe3)
@simaishi
Copy link
Contributor Author

Jansa backport details:

$ git log -1
commit 32006e2f98e3f74960db9a4847814a86720db9ff
Author: Jason Frey <[email protected]>
Date:   Thu Jun 11 14:30:39 2020 -0400

    Merge pull request #62 from simaishi/systemd_group

    Run bundle with systemd group

    (cherry picked from commit 9ebebe3d4c780316a99e000e90bbc38cce29fed4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working jansa/backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants