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 #24305 - container image registry config #7543

Merged
merged 1 commit into from
Aug 3, 2018

Conversation

thomasmckay
Copy link
Member

@thomasmckay thomasmckay commented Jul 19, 2018

@thomasmckay
Copy link
Member Author

@ekohl - matching PR to puppet-katello

@thomasmckay
Copy link
Member Author

[test katello]

url = cfg[:url]
if SETTINGS[:katello][:container_image_registry]
cfg = SETTINGS[:katello][:container_image_registry]
url = cfg[:crane_url]
Copy link
Member

Choose a reason for hiding this comment

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

This implies it only works with crane and no other container registry. Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ekohl - That's correct.

Copy link
Member

Choose a reason for hiding this comment

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

If it only works with crane, you might as well name the name the section crane, right? Registry is singular so a user wouldn't expect to be able to use more than one anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ekohl No, let's keep it as crane since that makes it very clear what service is being contacted. There is a chance there will be an alternative registry implementation in the coming months as well so this will help distinguish it. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought this had all been finalized on the related PR too. theforeman/puppet-katello#243 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to know where it's going and get it right the first time. I would expect registries to behave in the same way so they can be used interchangeable but perhaps this is not the reality.

So currently the format is:

:container_image_registry:
  :crane_url: ""
  :ca_ca_cert_file: ""

Now if another registry is added, how will this be configured?

:container_image_registry:
  :registry_type: "crane" # or my_registry
  :crane_url: ""
  :ca_ca_cert_file: ""
  :my_registry_type_url: ""
  :my_registry_type_ca_cert_file: ""

@thomasmckay
Copy link
Member Author

@ekohl - Ready to merge?

@akofink
Copy link
Member

akofink commented Jul 30, 2018

Perhaps it'd also be good to add this to katello.yaml.example.

@thomasmckay
Copy link
Member Author

@akofink - updated example

@thomasmckay thomasmckay merged commit bcc8ca9 into Katello:master Aug 3, 2018
@thomasmckay thomasmckay deleted the 24305-registry-config branch August 3, 2018 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants