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 evm_owner, tenant, group to orch stacks #288

Merged
merged 1 commit into from
Nov 12, 2018

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Oct 10, 2018

As part of the work for ManageIQ/manageiq#17951 (comment), orchestration stacks need an owner in order to be able to retire them as a request. Since we have group and tenant ids along with evm_owners, they're here too...

TODO: Things n stuff associated with this that still need to happen: we need to get both pathways sorted -- stacks that we discover from the providers need the user set, and ones we create ourselves need the user set at the end of provisioning.

As discussed with Madhu and GM, the approach of using the service owner and then the ems owner is the preferable way to do this.

@d-m-u
Copy link
Contributor Author

d-m-u commented Oct 10, 2018

@miq-bot add_label wip, hammer/yes

@d-m-u d-m-u force-pushed the adding_owner_to_orch_stacks branch 2 times, most recently from af3717f to 9545e77 Compare October 10, 2018 14:59
@d-m-u
Copy link
Contributor Author

d-m-u commented Oct 10, 2018

@miq-bot remove_label hammer/yes
good try though, drew

@d-m-u d-m-u force-pushed the adding_owner_to_orch_stacks branch 2 times, most recently from 6b8db66 to d4d08bf Compare October 15, 2018 20:16
@d-m-u d-m-u changed the title [WIP] Add evm_owner, tenant, group to orch stacks Add evm_owner, tenant, group to orch stacks Oct 16, 2018
@miq-bot miq-bot removed the wip label Oct 16, 2018
@d-m-u d-m-u force-pushed the adding_owner_to_orch_stacks branch 2 times, most recently from 644268e to 25919b9 Compare October 16, 2018 14:13
@d-m-u d-m-u changed the title Add evm_owner, tenant, group to orch stacks [WIP] Add evm_owner, tenant, group to orch stacks Oct 16, 2018
@miq-bot miq-bot added the wip label Oct 16, 2018
@d-m-u d-m-u force-pushed the adding_owner_to_orch_stacks branch 2 times, most recently from 77b5fc0 to 019f44f Compare October 16, 2018 14:31
@d-m-u d-m-u force-pushed the adding_owner_to_orch_stacks branch 5 times, most recently from f0dc611 to 126a640 Compare October 30, 2018 12:46
@d-m-u
Copy link
Contributor Author

d-m-u commented Oct 30, 2018

@miq-bot add_reviewer @tinaafitz

@miq-bot miq-bot requested a review from tinaafitz October 30, 2018 12:58
@d-m-u d-m-u force-pushed the adding_owner_to_orch_stacks branch 2 times, most recently from 2751fb1 to c1bf143 Compare October 30, 2018 13:12
@d-m-u d-m-u force-pushed the adding_owner_to_orch_stacks branch from 4b29f14 to 86d7c98 Compare November 12, 2018 20:54
@miq-bot
Copy link
Member

miq-bot commented Nov 12, 2018

Checked commit d-m-u@86d7c98 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🏆

@bdunne bdunne merged commit 550ba71 into ManageIQ:master Nov 12, 2018
@bdunne bdunne assigned bdunne and unassigned Fryguy Nov 12, 2018
@bdunne bdunne added this to the Sprint 99 Ending Nov 19, 2018 milestone Nov 12, 2018
@simaishi
Copy link
Contributor

@bdunne @Fryguy it's ok to backport this to hammer branch?

@bdunne
Copy link
Member

bdunne commented Nov 13, 2018

No, there are schema changes. I changed the labels accordingly.

@himdel
Copy link
Contributor

himdel commented Jul 24, 2019

Not sure if this is a Problem, but, this migration fails when updating a gaprindashvili db to ivanchuk:

== 20181016140921 MigrateOrchStacksToHaveOwnershipConcept: migrating ==========
-- Migrating existing orchestration stacks to have direct owners, groups, tenant
rails aborted!
StandardError: An error has occurred, this and all later migrations canceled:

10000000000001 is out of range for ActiveModel::Type::Integer with limit 4 bytes
/home/himdel/manageiq-ivanchuk/lib/extensions/ar_migration.rb:88:in `migrate'
bin/rails:4:in `require'
bin/rails:4:in `<main>'

Caused by:
ActiveModel::RangeError: 10000000000001 is out of range for ActiveModel::Type::Integer with limit 4 bytes
/home/himdel/manageiq-ivanchuk/lib/extensions/ar_migration.rb:88:in `migrate'
bin/rails:4:in `require'
bin/rails:4:in `<main>'
Tasks: TOP => db:migrate
(See full trace by running task with --trace)

@bdunne
Copy link
Member

bdunne commented Jul 24, 2019

Wow, what a coincidence, I was just looking into https://bugzilla.redhat.com/show_bug.cgi?id=1732808 which reports the same error. @himdel Do you have a reproducer environment?

@himdel
Copy link
Contributor

himdel commented Jul 24, 2019

Heheh, nice, I can share the db dump :) (edit: pm'd)

@jrafanie
Copy link
Member

jrafanie commented Jul 24, 2019

@bdunne @himdel What version of id_regions? maybe id_regions isn't 0.3.0? You'll get that error if an old version migration is run with rails 5.1+. This PR was needed to fix this:
ManageIQ/activerecord-id_regions#13

@himdel
Copy link
Contributor

himdel commented Jul 24, 2019

Hmmm, that could be it :)

I'm seeing 0.3.0 but running the migration again...

== 20181016140921 MigrateOrchStacksToHaveOwnershipConcept: migrating ==========
-- Migrating existing orchestration stacks to have direct owners, groups, tenant
   -> 15.2983s
== 20181016140921 MigrateOrchStacksToHaveOwnershipConcept: migrated (15.2983s) 

but I started my repo from hammer, so I guess the same bin/update that updated id_regions to 0.3.0 (from 0.2.2) then ran db:migrate, presumably still using the previous version, or something like that?

@jrafanie
Copy link
Member

oh wow... yeah, I can see that...

Dir.chdir(ManageIQ::Environment::APP_ROOT) do
  ManageIQ::Environment.ensure_config_files

  puts '== Installing dependencies =='
  ManageIQ::Environment.install_bundler
  ManageIQ::Environment.bundle_update  # updates id_regions

  ManageIQ::Environment.while_updating_ui do
    unless ENV["SKIP_DATABASE_SETUP"]
      ManageIQ::Environment.migrate_database # uses old bundle that launched bin/update
      ManageIQ::Environment.seed_database
    end

@bdunne
Copy link
Member

bdunne commented Jul 24, 2019

The bundle update completes before the database is migrated in bin/update, it's just updating javascript while raking / seeding

@himdel
Copy link
Contributor

himdel commented Jul 24, 2019

Well, migrate_database uses run_rake_task, which does system!.
So.. it is a new process.

However, bundler also rewrites RUBYLIB, PATH, GEM_PATH and GEM_HOME environment variables (and adds a bunch of BUNDLER_*) which do get propagated to the child process.

So... that could still be possible.. (I do remember having problems with ruby run from ruby when dealing with webpacker, so it does ring a bell, a bit :).)

Don't we have a clear bundle environment thing somewhere?

@himdel
Copy link
Contributor

himdel commented Jul 24, 2019

clear bundle environment thing

        Bundler.with_clean_env do
          system("bundle exec rake update:actual_ui")
        end

I think we would need that ^^ for db:migrate to work during the same bin/update.

@jrafanie
Copy link
Member

@himdel can you help me recreate it? Based on your information, I tried the following:

git checkout ivanchuk; git pull; bundle update; bin/rake db:migrate VERSION=20181023171353 
git checkout hammer; git pull; bin/update
git checkout ivanchuk; git pull; bin/update

The first step migrates the db back to the latest migration on hammer.
The second step gets to the dependencies at "hammer time" 🕺
The third step tries to bin/update from ivanchuk, which should bundle and migrate together.

I can't recreate it this way. Maybe I need something in my database in that table?

@jrafanie
Copy link
Member

Note, I put some findings into the bz... it looks like an issue of one migration running before another one polluting the memory space. If you migrate a second time, it works.

https://bugzilla.redhat.com/show_bug.cgi?id=1732808#c8

@himdel
Copy link
Contributor

himdel commented Jul 25, 2019

@jrafanie Sorry, apparenty not any more :(.

All I can tell you is that I took my manageiq hammer copy, updated to ivanchuk, and pointed it to a new database that came from a gaprindashvili copy I had lying around.

I can confirm that even if bin/update updates id.regions from 0.2.2 to 0.3.0 in the same run as running db:migrate, the migration passes. so, it was probably not that.

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

Successfully merging this pull request may close these issues.

10 participants