Add support for exporting and importing customization templates #17877
Conversation
branic
force-pushed
the
export_import_customization_templates
branch
from
0691671
to
18e06ee
Compare
| def validate_type(custom_template_type) | ||
| valid_types = CustomizationTemplate.descendants.collect(&:name) | ||
|
|
||
| valid_types.include?(custom_template_type) ? true : false |
There was a problem hiding this comment.
Rename method to valid_type?. Also, the include? method already returns true or false so no need for the ternary. This could be reduced to:
CustomizationTemplate.descendants.collect(&:name)include?(custom_template_type)Bonus Friday method example (not worth it since it makes the method longer, but wanted to show the example for future reference):
def valid_type?(custom_template_type)
klass = custom_template_type.safe_constantize
return false unless klass
klass < CustomizationTemplate
end| if pxe_image | ||
| { :pxe_image_type => pxe_image.to_model_hash.reject { |key| EXCLUDE_ATTRS.include?(key) } } | ||
| else | ||
| { :pxe_image_type => {} } |
There was a problem hiding this comment.
Is this something you ran into? Looks like it is an invalid configuration so we are just exporting something we know will fail to import.
Is there a reason to export or should the code detect and skip these records during export?
There was a problem hiding this comment.
This is to get around the issue that .to_model_hash outputs a key called :pxe_image_type_id and we need to convert that ID into the :name and provision_type from the PxeImageType that the ID is for so that the import can work between regions. The import script looks for a key called :pxe_image_type and coverts it into a valid PxeImageType object that is then used when finding the CustomiztionTemplate to update or creating a new CustomizationTemplate object.
Having the empty hash is there to support exporting the examples. The examples don't have a :pxe_image_type_id, but for the import to work I need to have the :pxe_image_type key so I just set it to an empty hash. The import script aborts with an error if the :pxe_image_type does not contain a key named :name.
|
|
||
| private | ||
|
|
||
| def pxe_image(pxe_image) |
There was a problem hiding this comment.
Suggest renaming to pxe_image_type_hash and pass pxe_image_type as the parameter.
| ct_hash = Exports.exclude_attributes(customization_template.to_model_hash, EXCLUDE_ATTRS) | ||
| ct_hash.merge!(pxe_image(customization_template.pxe_image_type)) | ||
|
|
||
| image_type_name = ct_hash[:pxe_image_type][:name] ? ct_hash[:pxe_image_type][:name] : "Examples" |
There was a problem hiding this comment.
Suggest: image_type_name = ct_hash.fetch_path(:pxe_image_type, :name) || "Examples"
| end | ||
|
|
||
| def assert_test_ct_one_present(pit) | ||
| # pit = PxeImageType.find_by(:name => existing_pit_name) |
There was a problem hiding this comment.
Probably don't need this line 😄
| let(:ct_desc) { "This template takes use of rootpassword defined in the UI" } | ||
| let(:existing_pit_name) { "RHEL-6" } | ||
| let(:new_pit_name) { "RHEL-7" } | ||
| let(:new_pit_pt) { "vm" } |
There was a problem hiding this comment.
This is kind of a minor quibble but a lot of these are only pertinent for one context, so I think they should be moved into that context. For example like the first one I noticed update_file and ct_system_file are only used in one context each (as are a few others), so they could be moved down into them.
Arguably, since they only really pertain to the #import method, even the top ones like data_dir should be moved inside the describe "#import" do block, but since that's all this class is supposed to do anyway, I don't think it's that big of a deal to have ones that are used all over at this high level.
|
|
||
| image_type_name = ct_hash[:pxe_image_type][:name] ? ct_hash[:pxe_image_type][:name] : "Examples" | ||
| filename = Exports.safe_filename("#{image_type_name}-#{ct_hash[:name]}", options[:keep_spaces]) | ||
| File.write("#{export_dir}/#{filename}.yaml", ct_hash.to_yaml) |
There was a problem hiding this comment.
Should we be timestamping these filenames at all?
There was a problem hiding this comment.
I don't believe so. In many (probably most) cases these scripts are being used to maintain the .yaml files in an SCM, Adding a timestamp to the filename would defeat the ability to see modifications over time.
There was a problem hiding this comment.
Also, none of the other export scripts add a timestamp to the filename.
|
Checked commits branic/manageiq@18e06ee~...28ab11a with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
These rake scripts and classes provide functionality for exporting/importing of the following ManageIQ object types:
This PR uses the framework that was implemented for PRs #14126, #15256, #16717, and others to export/import other ManageIQ object types.
These scripts are based on the CFME RH Consulting Scripts and are used by Red Hat consultants to enable storing customizations in Git and maintaining customizations between environments (e.g. dev/qa/prod) for an SDLC lifecycle.
Links [Optional]
Steps for Testing/QA [Optional]
Exporting
Importing