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

Refresh new target do not run post_refresh #16436

Merged
merged 1 commit into from
Nov 17, 2017
Merged

Conversation

pkliczewski
Copy link
Contributor

In case when we run refresh_new_target a vm is created before the
refresh was run so we need to make sure it is picked up by post_refresh.

Bug-Url:
https://bugzilla.redhat.com/1510459

@pkliczewski
Copy link
Contributor Author

@pkliczewski
Copy link
Contributor Author

@miq-bot add_label gaprindashvili/yes

@pkliczewski
Copy link
Contributor Author

@miq-bot assign @agrare

@@ -1092,7 +1092,7 @@ def refresh_ems_sync
end

def self.post_refresh_ems(ems_id, update_start_time)
update_start_time = update_start_time.utc
update_start_time = update_start_time.utc - 1.minute
Copy link
Member

Choose a reason for hiding this comment

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

This seems really hacky, lets see if there's a better way to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am open for suggestions. We use time to determine which the vm was created so I wonder what we can do here.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I'm thinking if we could try to get update_start_time from when refresh_new_target started, haven't checked if that is feasible yet

Choose a reason for hiding this comment

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

@agrare @pkliczewski any updates on your side?

Copy link
Member

Choose a reason for hiding this comment

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

@pkliczewski could you just call #post_create_actions_queue from refresh_new_target if the vm was created instead of messing with the ems_refresh_start_time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agrare will try

@agrare
Copy link
Member

agrare commented Nov 16, 2017

@pkliczewski this looks much cleaner, did you confirm it works?

If you make these changes the specs are passing locally for me:

   context '.refresh_new_target' do
-    let(:ems) { FactoryGirl.create(:ems_vmware) }
+    let(:ems) do
+      _, _, zone = EvmSpecHelper.create_guid_miq_server_zone
+      FactoryGirl.create(:ems_vmware, :zone => zone)
+    end

@pkliczewski
Copy link
Contributor Author

@agrare I checked that there are no failures. Will update the spec.

In case when we run refresh_new_target a vm is created before the
refresh was run so we need to make sure it is picked up by post_refresh.

Bug-Url:
https://bugzilla.redhat.com/1510459
@miq-bot
Copy link
Member

miq-bot commented Nov 17, 2017

Checked commit pkliczewski@4284854 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@agrare agrare merged commit 013c7b4 into ManageIQ:master Nov 17, 2017
simaishi pushed a commit that referenced this pull request Nov 17, 2017
Refresh new target do not run post_refresh
(cherry picked from commit 013c7b4)

https://bugzilla.redhat.com/show_bug.cgi?id=1514509
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 0c6620d30ba31b36f92147bd1ae17da2c2349595
Author: Adam Grare <[email protected]>
Date:   Fri Nov 17 09:26:03 2017 -0500

    Merge pull request #16436 from pkliczewski/master
    
    Refresh new target do not run post_refresh
    (cherry picked from commit 013c7b4cd84c54a0d8e771da5b77fcdc09b195a4)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1514509

@agrare agrare added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 27, 2017
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.

5 participants