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

Fixes #26047 - Add check for puppet in cvv export. #632

Merged
merged 1 commit into from Mar 4, 2019
Merged

Fixes #26047 - Add check for puppet in cvv export. #632

merged 1 commit into from Mar 4, 2019

Conversation

chris1984
Copy link
Member

This PR adds a check to export, to check and see if a cv version contains puppet modules and give the user a friendly error instead of having tar complain with something that is confusing.

I also re-factored the export checks into a method in the helper file to clean up the code within the content_view_version file to allow for more readability and flexibility for adding more checks etc that come up over time.

CV with only Puppet modules

[vagrant@centos7-katello-devel ~]$ hammer content-view version export --id 2 --export-dir /tmp
Could not export the content view:
  Error: The Content View 'puppet-view' contains Puppet modules, this is not supported at this time. Please remove the modules, publish a new version and try the export again.

CV with Puppet modules and yum repos

[vagrant@centos7-katello-devel ~]$ hammer content-view version export --id 4 --export-dir /tmp
Could not export the content view:
  Error: The Content View 'mixed-zoo' contains Puppet modules, this is not supported at this time. Please remove the modules, publish a new version and try the export again.

Verified checks still work after moving them to the helper file

[vagrant@centos7-katello-devel ~]$ hammer content-view version export --id 6 --export-dir /tmp
Could not export the content view:
  Error: All exported repositories must be set to an immediate download policy and re-synced.
  The following repositories need action:
    background
[vagrant@centos7-katello-devel ~]$ hammer content-view version export --id 7 --export-dir /tmp
Could not export the content view:
  Error: The Repository 'filerepo' is a non-yum repository. Only Yum is supported at this time. Please remove the repository from the Content View, republish and try the export again.

@theforeman-bot
Copy link

Issues: #26047

Copy link
Member

@akofink akofink left a comment

Choose a reason for hiding this comment

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

Seems pretty good (I didn't test it, but your tests look good also). This almost seems redundant, but I can see that a CV that doesn't contain any puppet repos can still have puppet modules published in a CVV. I just have a nitpick about the code style.

@chris1984
Copy link
Member Author

@akofink updated and addressed your comments. Confirmed tests still work:

[vagrant@centos7-katello-devel ~]$ hammer content-view version list
---|-------------------------------|---------|-----------------------
ID | NAME                          | VERSION | LIFECYCLE ENVIRONMENTS
---|-------------------------------|---------|-----------------------
7  | file-view 1.0                 | 1.0     | Library               
6  | background 1.0                | 1.0     | Library               
5  | on-demand 1.0                 | 1.0     | Library               
4  | mixed-zoo 1.0                 | 1.0     | Library               
3  | zoo 1.0                       | 1.0     | Library               
2  | puppet-view 1.0               | 1.0     | Library               
1  | Default Organization View 1.0 | 1.0     | Library               
---|-------------------------------|---------|-----------------------
[vagrant@centos7-katello-devel ~]$ hammer content-view version export --id 2 --export-dir /tmp
Could not export the content view:
  Error: The Content View 'puppet-view' contains Puppet modules, this is not supported at this time. Please remove the modules, publish a new version and try the export again.
[vagrant@centos7-katello-devel ~]$ hammer content-view version export --id 4 --export-dir /tmp
Could not export the content view:
  Error: The Content View 'mixed-zoo' contains Puppet modules, this is not supported at this time. Please remove the modules, publish a new version and try the export again.
[vagrant@centos7-katello-devel ~]$ hammer content-view version export --id 5 --export-dir /tmp
Could not export the content view:
  Error: All exported repositories must be set to an immediate download policy and re-synced.
  The following repositories need action:
    medium
[vagrant@centos7-katello-devel ~]$ hammer content-view version export --id 7 --export-dir /tmp
Could not export the content view:
  Error: The Repository 'filerepo' is a non-yum repository. Only Yum is supported at this time. Please remove the repository from the Content View, republish and try the export again.

Copy link
Member

@akofink akofink left a comment

Choose a reason for hiding this comment

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

Very nice. Thanks @chris1984 - ACK from me! I'd like another reviewer that's been working on this import/export work to review as well.

@chris1984
Copy link
Member Author

@ehelms care to review when you get a moment?

@ehelms ehelms merged commit 1abe582 into Katello:master Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants