Fixes #35215 - Handle cloned hostgroups in hosts_and_hostgroups_helper #10894
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
jeremylenz
force-pushed
the
35215-hostgroup-clone-handling-katello
branch
from
67f85c4
to
7ef5448
Compare
|
[test katello] |
jeremylenz
force-pushed
the
35215-hostgroup-clone-handling-katello
branch
from
7ef5448
to
c611daf
Compare
|
rebased |
|
In my testing this is still needed even after the changes to theforeman/foreman#10055. |
qcjames53
pushed a commit
to qcjames53/katello
that referenced
this pull request
Mar 19, 2024
Katello#10894) (cherry picked from commit e3d46c6)
qcjames53
added a commit
that referenced
this pull request
Mar 19, 2024
* Refs #37148 - Remove removed_widgets override (#10927) (cherry picked from commit 40a70ce) * Fixes #35215 - Handle cloned hostgroups in hosts_and_hostgroups_helper (#10894) (cherry picked from commit e3d46c6) * Fixes #36979 - Remove cdn_ssl_version setting This was originally added to allow downgrading the CDN connection SSL version for compatibility with much older proxy servers. That should be less of a concern now. We do still set a value of TLS v1.2 for the min_version, but only because ruby/openssl#709 prevents using the system-wide crypto policy for now. In the future, that can be removed as well, restoring control to the user at the OS level. (cherry picked from commit 78fcba9) * Fixes #37277 - Fix ACS randomly failing VCR tests (#10941) (cherry picked from commit 6d93801) * Fixes #37240 - Fix CCV duplicate repo warning (#10928) (cherry picked from commit 02fc313) --------- Co-authored-by: Jeremy Lenz <jlenz@redhat.com> Co-authored-by: William Bradford Clark <wclark@redhat.com> Co-authored-by: Ian Ballou <ianballou67@gmail.com> Co-authored-by: Markus Bucher <bucher@atix.de>
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.
Requires theforeman/foreman#10055
What are the changes introduced in this pull request?
When cloning a hostgroup, all existing hostgroup attributes should be pre-filled on the new form. However, some fields were not being pre-populated, including
There were two reasons for this.
First, I discovered that when a hostgroup is cloned, its content facet was not also being cloned. Because of this, only the "main" attributes were being copied over, and not the content facet attributes above. That should be addressed for all facets, not just content, so that fix is in theforeman/foreman#10055.
Second,
host_and_hostgroups_helperhas a few methods to fetch the existing values:These methods were returning
nilfor the newly-cloned hostgroup. I think this is because they relied on Rails associations which assume all the records are already saved in the database. The problem with cloned hostgroups is that the record is a new record, and is not yet saved in the database. For example, when callinghost_or_hostgroup.lifecycle_environment(assuminghost_or_hostgroupis a::Hostgroup), Rails normally looks at its associations:Rails is probably trying to look up the content facet via its ID, so that it can then retrieve the lifecycle environment. But since the content facet is a new record and does not have an ID, it would not be able to do this. I got around this by calling
host_or_hostgroup.content_facet.lifecycle_environmentdirectly, which seems to work.I also made similar changes for content view and content source.
Considerations taken when implementing this change?
What are the testing steps for this pull request?
Clone a hostgroup and verify that content source, lifecycle environment, and content view values are properly cloned
Save the cloned hostgroup and verify everything works as expected