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 #37865 - Add Multi environments to activation key info #958

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

chris1984
Copy link
Member

@chris1984 chris1984 commented Oct 1, 2024

With PR with an ak that has multiple envs:

[vagrant@devel hammer-cli-katello]$ hammer activation-key info --id 36
Name:                           foo-env
Id:                             36
Host Limit:                     0 of Unlimited
Multi Content View Environment: yes
Organization:
 1) Id:   1
    Name: Default Organization
Content view environments:
 1) Content view:
        Id:                      2
        Name:                    Mixed
        Version:                 1.0
        Content view version Id: 2
        Composite:               no
    Lifecycle environment:
        Id:   1
        Name: Library
    Candlepin Name:        Library/Mixed
 2) Content view:
        Id:                      3
        Name:                    Animals
        Version:                 1.0
        Content view version Id: 3
        Composite:               no
    Lifecycle environment:
        Id:   2
        Name: Dev
    Candlepin Name:        Dev/Animals

AK with single env:

[vagrant@devel hammer-cli-katello]$ hammer activation-key info --id 35
Name:                           foo
Id:                             35
Host Limit:                     0 of Unlimited
Multi Content View Environment: no
Organization:
 1) Id:   1
    Name: Default Organization
Content view environments:
 1) Content view:
        Id:                      2
        Name:                    Mixed
        Version:                 1.0
        Content view version Id: 2
        Composite:               no
    Lifecycle environment:
        Id:   1
        Name: Library
    Candlepin Name:        Library/Mixed

@jeremylenz
Copy link
Member

jeremylenz commented Oct 3, 2024

What do you think about adding another field like this?

Content View Environments: Library/Mixed,Dev/Animals

This would provide an easy way to copy/paste the current CVEs somewhere so you could edit as needed. It would also need another rabl node added in Katello.

Also it struck me as a bit weird that we don't provide each CVE's id anywhere. do you think we should add that as well? This way it could help with hammer activation-key update --content-view-environment-ids xxx

@chris1984
Copy link
Member Author

What do you think about adding another field like this?

Content View Environments: Library/Mixed,Dev/Animals

This would provide an easy way to copy/paste the current CVEs somewhere so you could edit as needed. It would also need another rabl node added in Katello.

Also it struck me as a bit weird that we don't provide each CVE's id anywhere. do you think we should add that as well? This way it could help with hammer activation-key update --content-view-environment-ids xxx

+1 to those ideas, when you say another rabl node would it go in https://github.com/Katello/katello/blob/bc39167a85e8aaa3ab4285bd459e672c7bcfc7a5/app/views/katello/api/v2/activation_keys/base.json.rabl or somewhere else? It sounds like this is related to that other card

@jeremylenz
Copy link
Member

jeremylenz commented Oct 7, 2024

would it go in https://github.com/Katello/katello/blob/bc39167a85e8aaa3ab4285bd459e672c7bcfc7a5/app/views/katello/api/v2/activation_keys/base.json.rabl or somewhere else?

Yes that's the file where it would go. It would be a top-level node.

I'm realizing we already have a node called :content_view_environments which is nested, so it'd have to be called something else. Maybe :content_view_environment_labels

@parthaa
Copy link
Contributor

parthaa commented Oct 8, 2024

+1 to those ideas, when you say another rabl node would it go in https://github.com/Katello/katello/blob/bc39167a85e8aaa3ab4285bd459e672c7bcfc7a5/app/views/katello/api/v2/activation_keys/base.json.rabl or somewhere else? It sounds like this is related to that other card

Doesnt need to be in RABL why not just in the info command?

@chris1984
Copy link
Member Author

@jeremylenz updated this pr and the Katello one, when you get time to test it

@jeremylenz
Copy link
Member

jeremylenz commented Oct 9, 2024

When I do hammer activation-key list I still see

---|--------|----------------|-----------------------|-------------
ID | NAME   | HOST LIMIT     | LIFECYCLE ENVIRONMENT | CONTENT VIEW
---|--------|----------------|-----------------------|-------------
1  | ak1    | 1 of Unlimited | Library               | cv1         
4  | ak3    | 0 of Unlimited | Library               | cv1         
3  | akZero | 0 of Unlimited |                       |             
---|--------|----------------|-----------------------|-------------

I think we should replace the lifecycle environment and content view columns with 'Content view environments' and use the new field you made. Should also add the 'Multi content view environment' field.

lib/hammer_cli_katello/activation_key.rb Outdated Show resolved Hide resolved
lib/hammer_cli_katello/activation_key.rb Outdated Show resolved Hide resolved
@chris1984
Copy link
Member Author

chris1984 commented Oct 15, 2024

@jeremylenz updated! Fixed the list command too. Added width 300 to fix long lists of CONTENT VIEW ENVIRONMENTS

---|-----------|----------------|----------------------------|-------------------------------
ID | NAME      | HOST LIMIT     | CONTENT VIEW ENVIRONMENTS  | MULTI CONTENT VIEW ENVIRONMENT
---|-----------|----------------|----------------------------|-------------------------------
3  | multi-key | 0 of Unlimited | Library/RHEL-8,Dev/Animals | yes
2  | RHEL-9    | 1 of Unlimited | Library/RHEL-9             | no
1  | Rocky-8   | 1 of Unlimited | Library/Rocky-8            | no
---|-----------|----------------|----------------------------|-------------------------------

Had to update the license because GPL-3.0 is deprecated and was causing Ruby 3.3 to not build.

Also enabled a skipped test that has been fixed since katello 4.5
Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@chris1984 chris1984 merged commit 2142e03 into Katello:main Oct 21, 2024
12 checks passed
@chris1984 chris1984 deleted the multipleenv-ak branch October 21, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants