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.

Sign up for GitHub

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

Jump to bottom

PXE Images refresh duplicates entries #18940

Closed
matejart opened this issue Jul 3, 2019 · 4 comments · Fixed by ManageIQ/manageiq-api#644, #19126, #19134 or ManageIQ/manageiq-api#707
Closed

PXE Images refresh duplicates entries #18940

matejart opened this issue Jul 3, 2019 · 4 comments · Fixed by ManageIQ/manageiq-api#644, #19126, #19134 or ManageIQ/manageiq-api#707
Assignees
@miha-plesko
Labels
bug

Comments

@matejart
Copy link

matejart commented Jul 3, 2019

Refreshing the relationships on an existing PXE server after a rename adds the images that have been added in previous refreshes. I took the following steps:

  • Using Compute > Infrastructure > PXE I visitied an existing PXE server
  • I observed that the PXE Images entries are correct. E.g., image01, image02, image03.
  • I used Configuration > Edit this PXE Server and changed the name of the PXE server.
  • I then used Configuration > Refresh Relationships
  • After several seconds, I refreshed the browser page.
  • The PXE Images list now reads: image01, image01, image02, image02, image03, image03.

The expected outcome would be that even after a renaming and refreshing, the list of images should be unchanged.
@matejart
Copy link
Author

matejart commented Jul 3, 2019

@miq-bot assign @miha-plesko

@matejart
Copy link
Author

matejart commented Jul 3, 2019

/cc @tadeboro

@tadeboro
Copy link
Contributor

tadeboro commented Jul 3, 2019

There are at least two bugs in the sources that cause this issue.

The first one is in the UI component of the MIQ. When the user updates any of the fields of the PXE server, all associated PXE menus are deleted and then recreated from the data in the form.

The second bug is a bit subtler and has to do with the database modeling around the PXE stuff. Currently, this is how we model the PXE servers, menus, and images:
server-menu-image

PXE images are only deleted from the database if we remove the parent server. On the other hand, if we remove a PXE menu, no image gets deleted.

Combine both of the bugs, and we have got our issue at hand.

If we would like to solve this The Right Way (TM), we will probably need to remove the pxe_server_id foreign key from the pxe_images table, set some :dependent options on the models in question, and make sure we only destroy PXE menus when the location of the menu file changes.

@Fryguy
Copy link
Member

Fryguy commented Aug 3, 2019

cc @bdunne

@Fryguy Fryguy added the bug label Aug 3, 2019
miha-plesko added a commit to xlab-si/manageiq-api that referenced this issue Aug 8, 2019
@miha-plesko
Destroy pxe_menu images upon removal
0b1931c 
We were invoking `.delete_all` method (aliased as `.clear`)
which avoids `:dependent => destroy` callbacks hence related PXE images
remained in database.

Fixes ManageIQ/manageiq#18940

Signed-off-by: Miha Pleško <[email protected]>
@miha-plesko miha-plesko mentioned this issue Aug 8, 2019
Merged
miha-plesko added a commit to xlab-si/manageiq that referenced this issue Aug 9, 2019
@miha-plesko
Move relation into the base class
c62914e 
When one destroys PxeMenu, also related MiqImages are destroyed.
This works both subclasses of PxeMenu:

- `PxeMenuIpxe`
- `PxeMenuPxelinux`

but does not work for base class, which is annoying in rspecs.
With this commit we simply move relation definition to the
base class so that it now works everywhere.

Fixes ManageIQ#18940

Signed-off-by: Miha Pleško <[email protected]>
miha-plesko added a commit to xlab-si/manageiq-api that referenced this issue Aug 9, 2019
@miha-plesko
Protect user work
53aaae2 
Currently, when user updates PXE Server details and submits
form, all related PxeMenus and PxImages are discarded so she
needs to run "Recreate Relationships" to inventory them again.

This approach works but unfortunately user has option to manually
edit PxeImages when they are inventoried, and by discarding
them we also discard her work.

With this commit we therefore go out of our way and double
check which PxeMenus (and subsequent PxeImages) are *really*
affected by update and only discard those.

This way, if user only updates PxeServer's name, nothing else
will be recreated hence user's manual work will be preserved.

Fixes ManageIQ/manageiq#18940

Signed-off-by: Miha Pleško <[email protected]>
miha-plesko added a commit to xlab-si/manageiq-api that referenced this issue Aug 9, 2019
@miha-plesko
Protect user's work
caaa978 
Currently, when user updates PXE Server details and submits
form, all related PxeMenus and PxImages are discarded so she
needs to run "Recreate Relationships" to inventory them again.

This approach works but unfortunately user has option to manually
edit PxeImages when they are inventoried, and by discarding
them we also discard her work.

With this commit we therefore go out of our way and double
check which PxeMenus (and subsequent PxeImages) are *really*
affected by update and only discard those.

This way, if user only updates PxeServer's name, nothing else
will be recreated hence user's manual work will be preserved.

Fixes ManageIQ/manageiq#18940

Signed-off-by: Miha Pleško <[email protected]>
This was referenced Aug 9, 2019
Closed
Merged
miha-plesko added a commit to xlab-si/manageiq that referenced this issue Aug 12, 2019
@miha-plesko
Utility function to update PxeMenu list
e1afbe0 
With this commit we introduce a utility method on PxeServer model
to handle PxeMenus list update. Until now, miq-api was always
recreating all menus upon server update, but often we can actually
just preserve exisitng menus. This method calculates what menus to
add, what to preserve and what to remove, and then executes it.

Fixes ManageIQ#18940

Signed-off-by: Miha Pleško <[email protected]>
@miha-plesko miha-plesko mentioned this issue Aug 12, 2019
Merged
miha-plesko added a commit to xlab-si/manageiq-api that referenced this issue Aug 12, 2019
@miha-plesko
Protect user's work
c30aa65 
Currently, when user updates PXE Server details and submits
form, all related PxeMenus and PxImages are discarded so she
needs to run "Recreate Relationships" to inventory them again.

This approach works but unfortunately user has option to manually
edit PxeImages when they are inventoried, and by discarding
them we also discard her work.

With this commit we therefore go out of our way and double
check which PxeMenus (and subsequent PxeImages) are *really*
affected by update and only discard those.

This way, if user only updates PxeServer's name, nothing else
will be recreated hence user's manual work will be preserved.

Fixes ManageIQ/manageiq#18940

Signed-off-by: Miha Pleško <[email protected]>
miha-plesko added a commit to xlab-si/manageiq that referenced this issue Aug 13, 2019
@miha-plesko
Utility function to update PxeMenu list
9e1366a 
With this commit we introduce a utility method on PxeServer model
to handle PxeMenus list update. Until now, miq-api was always
recreating all menus upon server update, but often we can actually
just preserve exisitng menus. This method calculates what menus to
add, what to preserve and what to remove, and then executes it.

Fixes ManageIQ#18940

Signed-off-by: Miha Pleško <[email protected]>
thearifismail pushed a commit to thearifismail/manageiq that referenced this issue Aug 21, 2019
@miha-plesko @thearifismail
Move relation into the base class
f92e637 
When one destroys PxeMenu, also related MiqImages are destroyed.
This works both subclasses of PxeMenu:

- `PxeMenuIpxe`
- `PxeMenuPxelinux`

but does not work for base class, which is annoying in rspecs.
With this commit we simply move relation definition to the
base class so that it now works everywhere.

Fixes ManageIQ#18940

Signed-off-by: Miha Pleško <[email protected]>
thearifismail pushed a commit to thearifismail/manageiq-api that referenced this issue Aug 22, 2019
@miha-plesko @thearifismail
Destroy pxe_menu images upon removal
4bdd441 
We were invoking `.delete_all` method (aliased as `.clear`)
which avoids `:dependent => destroy` callbacks hence related PXE images
remained in database.

Fixes ManageIQ/manageiq#18940

Signed-off-by: Miha Pleško <[email protected]>
martinpovolny pushed a commit to martinpovolny/manageiq-api that referenced this issue Nov 11, 2019
@miha-plesko @martinpovolny
Protect user's work
378e79e 
Currently, when user updates PXE Server details and submits
form, all related PxeMenus and PxImages are discarded so she
needs to run "Recreate Relationships" to inventory them again.

This approach works but unfortunately user has option to manually
edit PxeImages when they are inventoried, and by discarding
them we also discard her work.

With this commit we therefore go out of our way and double
check which PxeMenus (and subsequent PxeImages) are *really*
affected by update and only discard those.

This way, if user only updates PxeServer's name, nothing else
will be recreated hence user's manual work will be preserved.

Fixes ManageIQ/manageiq#18940

Signed-off-by: Miha Pleško <[email protected]>
@martinpovolny martinpovolny mentioned this issue Nov 11, 2019
Closed
martinpovolny pushed a commit to martinpovolny/manageiq-api that referenced this issue Nov 11, 2019
@miha-plesko @martinpovolny
Protect user's work
3a83a31 
Currently, when user updates PXE Server details and submits
form, all related PxeMenus and PxImages are discarded so she
needs to run "Recreate Relationships" to inventory them again.

This approach works but unfortunately user has option to manually
edit PxeImages when they are inventoried, and by discarding
them we also discard her work.

With this commit we therefore go out of our way and double
check which PxeMenus (and subsequent PxeImages) are *really*
affected by update and only discard those.

This way, if user only updates PxeServer's name, nothing else
will be recreated hence user's manual work will be preserved.

Fixes ManageIQ/manageiq#18940

Signed-off-by: Miha Pleško <[email protected]>
martinpovolny pushed a commit to martinpovolny/manageiq-api that referenced this issue Nov 11, 2019
@miha-plesko @martinpovolny
Protect user's work
902b6c7 
Currently, when user updates PXE Server details and submits
form, all related PxeMenus and PxImages are discarded so she
needs to run "Recreate Relationships" to inventory them again.

This approach works but unfortunately user has option to manually
edit PxeImages when they are inventoried, and by discarding
them we also discard her work.

With this commit we therefore go out of our way and double
check which PxeMenus (and subsequent PxeImages) are *really*
affected by update and only discard those.

This way, if user only updates PxeServer's name, nothing else
will be recreated hence user's manual work will be preserved.

Fixes ManageIQ/manageiq#18940

Signed-off-by: Miha Pleško <[email protected]>
@martinpovolny martinpovolny mentioned this issue Nov 11, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@miha-plesko miha-plesko

Labels
bug
Projects
None yet
Milestone
No milestone
4 participants
@Fryguy @tadeboro @matejart @miha-plesko