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.

Sign up for GitHub

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

Jump to bottom

Azure labeling and tagging support #3697

Merged
merged 1 commit into from
Apr 18, 2018
Merged

Azure labeling and tagging support #3697

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions app/controllers/ops_controller/settings/label_tag_mapping.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module OpsController::Settings::LabelTagMapping
extend ActiveSupport::Concern

Expand All @@ -13,6 +15,7 @@ module OpsController::Settings::LabelTagMapping
MAPPABLE_ENTITIES = {
nil => nil, # map all entities
"Vm" => "Amazon",
"VmAzure" => "Azure",
"Image" => "Amazon",
"ContainerProject" => "Kubernetes",
"ContainerRoute" => "Kubernetes",
Expand Down Expand Up @@ -79,7 +82,7 @@ def entity_ui_name_or_all(entity)
if entity
provider = MAPPABLE_ENTITIES[entity]
model = case entity
when 'Vm'
when 'Vm', 'VmAzure'
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that this break the pluggable providers approach. But it seems to be the case before, so if @martinpovolny is ok with this, I guess we can merge.

We should refactor this though next, so this generic code doesn't know anything about k8s, AWS or Azure

Copy link
Member

@martinpovolny martinpovolny Apr 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I am "ok" with that given this is a blocker BZ.

Code like this should not exist in the UI. The UI should never test for "azure" or anything like that. We should only use the supports? interface from the UI.

I am going to merge this PR, but with the expectation that there's going to be a follow up that fixes this and this too:

provider.azure? && entity.sub(/Azure$/, '...

Copy link
Contributor

Choose a reason for hiding this comment

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

See proposed refactor followup: 22a532f
(might also help reviewing this PR — the explicit values there were computed by actually running logic from entity_ui_name_or_all and label_tag_mapping_add in this PR).

It still won't be "pluggable", there is currently no mechanism by which providers report which types can be mapped in their refresh (and UI shouldn't offer types that wouldn't work).

I'm happy to later do a bigger followup with a migration to a better schema, but I'm afraid the answer at this point will be "don't bother", justifiably so...

"ManageIQ::Providers::#{provider}::CloudManager::Vm"
when 'Image'
"ManageIQ::Providers::#{provider}::CloudManager::Template"
Expand Down Expand Up @@ -172,8 +175,8 @@ def label_tag_mapping_add(entity, label_name, cat_description)
provider = ALL_PREFIX
entity_str = ''
else
provider = MAPPABLE_ENTITIES[entity].downcase
entity_str = entity.underscore
provider = MAPPABLE_ENTITIES[entity].downcase.inquiry
entity_str = (provider.azure? && entity.sub(/Azure$/, '') || entity).underscore
end

cat_name = "#{provider}:#{entity_str}:" + Classification.sanitize_name(label_name.tr("/", ":"))
Expand All @@ -192,8 +195,9 @@ def label_tag_mapping_add(entity, label_name, cat_description)
:description => cat_description,
:single_value => true,
:read_only => true)
ContainerLabelTagMapping.create!(:labeled_resource_type => entity, :label_name => label_name,
:tag => category.tag)
ContainerLabelTagMapping.create!(:labeled_resource_type => entity,
:label_name => label_name,
:tag => category.tag)
end
rescue StandardError => bang
add_flash(_("Error during 'add': %{message}") % {:message => bang.message}, :error)
Expand Down