Add type to ContainerTemplate to allow subclassing.

[lfu]

Part of refactoring kube/openshift specific template code into its own provider directory.

Part of ManageIQ/manageiq#15523.

@miq-bot add_label refactoring

[gmcculloug]

@miq-bot assign @Fryguy

[Fryguy]

cc @simon3z @enoodle Does this overlap with something you all were working on?

[enoodle]

cc @simon3z @enoodle Does this overlap with something you all were working on?

It is similar to #21 but with template instead of image. One key difference is that we left ContainerImage in the same place and didn't move it under ManageIQ::Providers::ContainerManager.

[lfu]

@enoodle Do you mean ContainerImage should be refactored and moved into ManageIQ::Providers::ContainerManager?

[enoodle]

@lfu This is what I understand from this PR. Do you know if this is a requirement now that all Container models should be moved under ManageIQ::Poviders::ContainerManager? Why is template being moved in ManageIQ/manageiq#15523?
I feel that if we are moving the container models then it should be done in one PR, so maybe ManageIQ/manageiq#15523 should move all of them?

[bzwei]

should it be ManageIQ::Providers::OpenShift::ContainerManager::ContainerTemplate or derive the class name with actual provider name from its container manager?

[lfu]

I feel that if we are moving the container models then it should be done in one PR, so maybe ManageIQ/manageiq#15523 should move all of them?

@enoodle That is correct. ManageIQ/manageiq#15523 is aimed at moving all Kube/Openshift specific code into its provider directory. Let me know if I have missed any other file.

[gmcculloug]

@lfu Tests are failing because OpenShift here does not match the test for Openshift (lower case "s").

You want to use "Openshift". Example: https://github.com/ManageIQ/manageiq-providers-openshift/blob/master/app/models/manageiq/providers/openshift/container_manager.rb#L1

[miq-bot]

Checked commit lfu@e8552ce with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 1 offense detected

db/migrate/20170707150520_update_container_template_types.rb

• ❗ - Line 8, Col 25 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.

[gmcculloug]

@enoodle @Fryguy Anything else needed here to merge this?

[enoodle]

IMO if we want to separate all the container classes that have mostly Openshift functions then we need to also move: ContainerBuild, ContainerBuildPod, ContainerDeployment and ContainerRoute. The all only Openshift meaning (I guess Route will/should be changed to include kubernetes ingress soon).

[lfu]

@enoodle Opened an issue to refactor all other container classes to its provider directory.
This PR focuses on the template provisioning related refactoring. Is there any other related class should be added in this PR?

[enoodle]

@lfu I don't think there is anything else that is concerning templates.

[lfu]

@Fryguy Please review.

[Fryguy]

Gah I just merged this, but how do you know the container template is always openshift...what if it's kubernetes?

[lfu]

I thought Openshift template is the only container template that is currently supported.

[cben]

Yeah, templates are an Openshift concept, they don't exist in Kubernetes.
(they might in future, openshift concepts tend to get upstreamed, there is already a proposal)

Currently kubernetes refresh doesn't even try to collect/parse/create ContainerTemplate, this migration's assumption is safe. (aside from related PRs not being merged yet)