Fixes #36064 - Don't try to assign empty CV/LCE to a cloned host #10448
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Issues: #36064 |
jeremylenz
force-pushed
the
36064-unbreak-my-clones
branch
from
a117bb9
to
1149ae3
Compare
|
Hmmm, it does not seem to solve the issue for me. |
|
@ares What are your exact repro steps? I reproduced the error but I did notice the stack was a bit different. And I could only reproduce on a host without content in build mode. |
jeremylenz
force-pushed
the
36064-unbreak-my-clones
branch
from
1149ae3
to
eebdebf
Compare
|
@ares Updated. I wasn't able to reliably reproduce the error, but I think I've fixed the codepath from the error stack you posted. |
|
The UI issue we saw where the CV is not shown for the hostgroup even if it's present in the backend causes an incorrect patch request to go out with empty CV if we don't re-set it for every edit when using synced content from a kickstart repo in a CV. |
This is a separate issue and I'm hoping it will be corrected when #10380 is ready.. |
sjha4
approved these changes
Mar 15, 2023
There was a problem hiding this comment.
The UI issue we saw where the CV is not shown for the hostgroup even if it's present in the backend causes an incorrect patch request to go out
This is a separate issue and I'm hoping it will be corrected when #10380 is ready..
Ack..Remember or remind me to test this on that PR..
This PR looks good. Was able to reproduce the error and the stacktrace in different flows. This patch fixes it. 👍🏼
lfu
pushed a commit
to lfu/katello
that referenced
this pull request
Mar 16, 2023
…atello#10448) (cherry picked from commit 870746d)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are the changes introduced in this pull request?
Host cloning was broken! In
hosts_and_hostgroups_helper, thekickstart_repository_optionsmethod was taking the original host's content view and lifecycle environment and assigning it to the cloned host, without first checking to see if the original host even had a CVE assigned. Now, it skips cloning the CVE if the original host doesn't have one.Considerations taken when implementing this change?
Currently only works for single-environment hosts. I didn't cover multi-environment hosts because we haven't yet adapted anything else for multi-environment hosts. But we'll have to do that eventually.
For now, for a multi-environment host it just wouldn't clone the multiple CVEs and your cloned host would be left with no CVE. But at least you won't get a Rails error.
What are the testing steps for this pull request?
Try, from either the old or new host UI, to clone a host that doesn't have a LCE/CV assigned.