Pluggable provider mapping prefixes and targets #20776

lpichler · 2020-11-04T13:22:54Z

Fixes and adds supporting method to solve 2 points from #19440:

Tag Mapper tag prefixes https://github.com/ManageIQ/manageiq/blob/master/app/models/provider_tag_mapping.rb#L25
Tag mapper categories (https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/controllers/ops_controller/settings/label_tag_mapping.rb#L21-L28)

1. is fixed in this PR and 2. will be fixed in UI PR

Supported in providers:

Amazon Supports label mapping in cloud manager manageiq-providers-amazon#669
Azure Supports label mapping in cloud manager manageiq-providers-azure#424
IBM Supports label mapping in cloud manager manageiq-providers-ibm_cloud#116
Kubernetes Add label mapping target_class and prefix methods to container manager manageiq-providers-kubernetes#409
Openstack Supports label mapping in cloud manager manageiq-providers-openstack#665
Vmware Supports label mapping in infra manager manageiq-providers-vmware#669

Links

[Providers] Pluggability Checklist #19440

lpichler · 2020-11-04T13:25:01Z

Once review will be finished I will create rest of provider PRs

@miq-bot assign @agrare

app/models/ext_management_system.rb

agrare · 2020-11-04T20:44:11Z

app/models/ext_management_system.rb

+    supported_subclasses.select(&:supported_for_label_mapping?)
+  end
+
+  def self.supported_label_mapping_prefixes


Unless we're going to have an unsupported_label_mapping_prefixes we can probably just call this self.label_mapping_prefixes or all_label_mapping_prefixes

agrare · 2020-11-04T20:47:05Z

@lpichler should we include "/managed/" in the mapping prefix name? Either in the providers or as part of the label_mapping_prefixes method? Not sure if you need this list without the "/managed" prefix

agrare · 2020-11-04T20:50:45Z

@lpichler for the categories it looks like you might need more info than just the class names?
Not sure how you go from e.g. VmIBM to "ibm:vm:" and "ManageIQ::Providers::IbmCloud::VPC::CloudManager::Vm" generically.
Maybe we need to be updating those categories to be more consistent and able to be built automatically? Along these lines I did put in ManageIQ/manageiq-schema#525 to make Image/Vm more amazon specific

lpichler · 2020-11-05T11:16:06Z

@lpichler should we include "/managed/" in the mapping prefix name? Either in the providers or as part of the label_mapping_prefixes method? Not sure if you need this list without the "/managed" prefix

No. /managed is prefix which is related to our tags so I would like to leave it in; core. But in upcoming refactoring will most likely remove part map { |name| "/managed/#{name}:" } in ProviderTagMapping:25

@lpichler for the categories it looks like you might need more info than just the class names?
Not sure how you go from e.g. VmIBM to "ibm:vm:" and "ManageIQ::Providers::IbmCloud::VPC::CloudManager::Vm" generically.
Maybe we need to be updating those categories to be more consistent and able to be built automatically? Along these lines I did put in ManageIQ/manageiq-schema#525 to make Image/Vm more amazon specific

Yes, you are right we need it for ProviderTagMapping#labeled_resource_type so I change it to have also this.
See provider PRs for amazon and Kubernetes
I think we can find way how to build ProviderTagMapping#labeled_resource_type` automatically but let's leave on other PR.

@agrare you can re-review it,
thanks!

agrare · 2020-11-05T13:22:07Z

@miq-bot cross-repo-tests ManageIQ/manageiq-providers-amazon#669, ManageIQ/manageiq-providers-kubernetes#409, including ManageIQ/manageiq-providers-amazon#669 ManageIQ/manageiq-providers-kubernetes#409 ManageIQ/manageiq-schema#525 #20776

From Pull Request: ManageIQ/manageiq#20776

agrare

Looks good to me, cross repo is green, thanks @lpichler !

From Pull Request: ManageIQ/manageiq#20776

lpichler · 2020-11-05T17:16:39Z

From Pull Request: ManageIQ/manageiq#20776

lpichler · 2020-11-05T18:07:49Z

I am not sure what I am doing wrong for cross testing miq bot command:

@miq-bot cross-repo-tests https://github.com/ManageIQ/manageiq-providers-amazon/pull/669,https://github.com/ManageIQ/manageiq-providers-azure/pull/424,https://github.com/ManageIQ/manageiq-providers-ibm_cloud/pull/116,https://github.com/ManageIQ/manageiq-providers-kubernetes/pull/409,https://github.com/ManageIQ/manageiq-providers-openstack/pull/665,https://github.com/ManageIQ/manageiq-providers-vmware/pull/669 including https://github.com/ManageIQ/manageiq-providers-amazon/pull/669 https://github.com/ManageIQ/manageiq-providers-azure/pull/424 https://github.com/ManageIQ/manageiq-providers-ibm_cloud/pull/116 https://github.com/ManageIQ/manageiq-providers-kubernetes/pull/409 https://github.com/ManageIQ/manageiq-providers-openstack/pull/665 https://github.com/ManageIQ/manageiq-providers-vmware/pull/669 https://github.com/ManageIQ/manageiq-schema/pull/525 https://github.com/ManageIQ/manageiq/pull/20776

From Pull Request: ManageIQ/manageiq#20776

agrare · 2020-11-10T14:49:13Z

@miq-bot cross-repo-tests manageiq-ui-classic, manageiq-api, ManageIQ/manageiq-providers-amazon#669, ManageIQ/manageiq-providers-azure#424, ManageIQ/manageiq-providers-ibm_cloud#116, ManageIQ/manageiq-providers-kubernetes#409, ManageIQ/manageiq-providers-openstack#665, ManageIQ/manageiq-providers-vmware#669 including ManageIQ/manageiq-providers-amazon#669

agrare · 2020-11-10T14:49:42Z

Just going to do one more cross-repo test with the UI and API included

From Pull Request: ManageIQ/manageiq#20776

agrare · 2020-11-10T15:12:28Z

Currently only failing on an unrelated UI error

From Pull Request: ManageIQ/manageiq#20776

Fryguy · 2020-11-12T14:35:48Z

app/models/ext_management_system.rb

+
+  def self.entities_for_label_mapping
+    label_mapping_classes.reduce({}) { |all_mappings, klass| all_mappings.merge(klass.entities_for_label_mapping) }
+  end


Please mark any of the above as private if they are not to be used as public API of the class.

Can you also add specs for these methods?

I added specs but we need public API, at least for now for UI.

From Pull Request: ManageIQ/manageiq#20776

agrare · 2020-11-12T18:14:33Z

@lpichler you need to rebase this PR on master to fix that k8s test

From Pull Request: ManageIQ/manageiq#20776

lpichler · 2020-11-13T07:59:16Z

@miq-bot cross-repo-tests ManageIQ/manageiq-providers-amazon#669, ManageIQ/manageiq-providers-azure#424, ManageIQ/manageiq-providers-ibm_cloud#116, ManageIQ/manageiq-providers-kubernetes#409, ManageIQ/manageiq-providers-openstack#665, ManageIQ/manageiq-providers-vmware#669 including ManageIQ/manageiq-providers-amazon#669

From Pull Request: ManageIQ/manageiq#20776

lpichler · 2020-11-13T08:48:33Z

✅✅✅ All checks have passed in ManageIQ/manageiq-cross_repo-tests#235 ✅✅✅

agrare · 2020-11-13T20:00:08Z

spec/models/ext_management_system_spec.rb

+  }.freeze
+
+  it "supports label mapping for provider subclasses" do
+    expect(SUPPORTED_MAPPING_ENTITIES).to eq(ExtManagementSystem.entities_for_label_mapping)


Sorry @lpichler (just noticed you added this) but can you update this to check .includes on one or two values instead of .eq on the whole list? I don't want to get back into having the same problems as we fixed in #20595 aka everytime we add another one core specs start failing.

You don't need to rerun the whole cross repo tests again though :)

Sorry @lpichler (just noticed you added this) but can you update this to check .includes on one or two values instead of .eq on the whole list? I don't want to get back into having the same problems as we fixed in #20595 aka everytime we add another one core specs start failing.

Done 👍

You don't need to rerun the whole cross repo tests again though :)

👍 :)

thanks @agrare

miq-bot · 2020-11-16T18:30:28Z

Checked commit lpichler@a173322 with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint
4 files checked, 0 offenses detected
Everything looks fine. ⭐

lpichler requested review from agrare, Fryguy and kbrock as code owners November 4, 2020 13:22

miq-bot assigned agrare Nov 4, 2020

agrare reviewed Nov 4, 2020

View reviewed changes

app/models/ext_management_system.rb Outdated Show resolved Hide resolved

agrare reviewed Nov 4, 2020

View reviewed changes

lpichler force-pushed the pluggable_provider_mapping_prefixes_and_targets branch 2 times, most recently from c984ffb to c9f1679 Compare November 5, 2020 10:59

miq-bot mentioned this pull request Nov 5, 2020

[BOT] Cross repo test run ManageIQ/manageiq-cross_repo-tests#210

Closed

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 5, 2020

Running tests for agrare

4f902a9

From Pull Request: ManageIQ/manageiq#20776

agrare added a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 5, 2020

Running tests for agrare

1be76e7

From Pull Request: ManageIQ/manageiq#20776

agrare added a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 5, 2020

Running tests for agrare

f6bf0d8

From Pull Request: ManageIQ/manageiq#20776

agrare added a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 5, 2020

Running tests for agrare

d75c5ba

From Pull Request: ManageIQ/manageiq#20776

agrare approved these changes Nov 5, 2020

View reviewed changes

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 5, 2020

Running tests for agrare

c8fbce4

From Pull Request: ManageIQ/manageiq#20776

miq-bot mentioned this pull request Nov 5, 2020

[BOT] Cross repo test run ManageIQ/manageiq-cross_repo-tests#211

Closed

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 5, 2020

Running tests for lpichler

09f299e

From Pull Request: ManageIQ/manageiq#20776

miq-bot mentioned this pull request Nov 5, 2020

[BOT] Cross repo test run ManageIQ/manageiq-cross_repo-tests#212

Closed

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 5, 2020

Running tests for lpichler

c7f29ea

From Pull Request: ManageIQ/manageiq#20776

miq-bot mentioned this pull request Nov 5, 2020

[BOT] Cross repo test run ManageIQ/manageiq-cross_repo-tests#213

Closed

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 5, 2020

Running tests for lpichler

90830f8

From Pull Request: ManageIQ/manageiq#20776

miq-bot mentioned this pull request Nov 5, 2020

[BOT] Cross repo test run ManageIQ/manageiq-cross_repo-tests#214

Closed

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 5, 2020

Running tests for lpichler

6b23dc5

From Pull Request: ManageIQ/manageiq#20776

agrare approved these changes Nov 10, 2020

View reviewed changes

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 10, 2020

Running tests for agrare

44a5e40

From Pull Request: ManageIQ/manageiq#20776

miq-bot mentioned this pull request Nov 10, 2020

[BOT] Cross repo test for ManageIQ/manageiq#20776 ManageIQ/manageiq-cross_repo-tests#226

Closed

lpichler mentioned this pull request Nov 12, 2020

Add Instance (IBM) to en.yml #20800

Merged

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 12, 2020

Running tests for lpichler

6180cb3

From Pull Request: ManageIQ/manageiq#20776

miq-bot mentioned this pull request Nov 12, 2020

[BOT] Cross repo test for ManageIQ/manageiq#20776 ManageIQ/manageiq-cross_repo-tests#230

Closed

Fryguy reviewed Nov 12, 2020

View reviewed changes

lpichler force-pushed the pluggable_provider_mapping_prefixes_and_targets branch from c9f1679 to edf3531 Compare November 12, 2020 17:13

lpichler requested a review from gtanzillo as a code owner November 12, 2020 17:13

lpichler force-pushed the pluggable_provider_mapping_prefixes_and_targets branch from edf3531 to a011707 Compare November 12, 2020 17:16

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 12, 2020

Running tests for lpichler

f600148

From Pull Request: ManageIQ/manageiq#20776

miq-bot mentioned this pull request Nov 12, 2020

[BOT] Cross repo test for ManageIQ/manageiq#20776 ManageIQ/manageiq-cross_repo-tests#231

Closed

lpichler force-pushed the pluggable_provider_mapping_prefixes_and_targets branch from a011707 to 3a7f053 Compare November 13, 2020 06:59

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 13, 2020

Running tests for lpichler

31f9128

From Pull Request: ManageIQ/manageiq#20776

miq-bot mentioned this pull request Nov 13, 2020

[BOT] Cross repo test for ManageIQ/manageiq#20776 ManageIQ/manageiq-cross_repo-tests#234

Closed

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Nov 13, 2020

Running tests for lpichler

ca3ef8c

From Pull Request: ManageIQ/manageiq#20776

miq-bot mentioned this pull request Nov 13, 2020

[BOT] Cross repo test for ManageIQ/manageiq#20776 ManageIQ/manageiq-cross_repo-tests#235

Closed

agrare reviewed Nov 13, 2020

View reviewed changes

Add methods for provider mapping supportability in ExtManagementSystem

a173322

lpichler force-pushed the pluggable_provider_mapping_prefixes_and_targets branch from 3a7f053 to a173322 Compare November 16, 2020 18:26

agrare approved these changes Nov 18, 2020

View reviewed changes

agrare merged commit 267ceed into ManageIQ:master Nov 18, 2020

agrare mentioned this pull request Nov 19, 2020

[Providers] Pluggability Checklist #19440

Open

48 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pluggable provider mapping prefixes and targets #20776

Pluggable provider mapping prefixes and targets #20776

lpichler commented Nov 4, 2020 •

edited by agrare

Loading

lpichler commented Nov 4, 2020

agrare Nov 4, 2020

lpichler Nov 5, 2020

agrare commented Nov 4, 2020

agrare commented Nov 4, 2020

lpichler commented Nov 5, 2020

agrare commented Nov 5, 2020 •

edited

Loading

agrare left a comment

lpichler commented Nov 5, 2020

lpichler commented Nov 5, 2020

agrare commented Nov 10, 2020 •

edited

Loading

agrare commented Nov 10, 2020

agrare commented Nov 10, 2020

Fryguy Nov 12, 2020

Fryguy Nov 12, 2020

lpichler Nov 12, 2020

agrare commented Nov 12, 2020

lpichler commented Nov 13, 2020

lpichler commented Nov 13, 2020

agrare Nov 13, 2020

lpichler Nov 16, 2020

miq-bot commented Nov 16, 2020

Pluggable provider mapping prefixes and targets #20776

Pluggable provider mapping prefixes and targets #20776

Conversation

lpichler commented Nov 4, 2020 • edited by agrare Loading

Links

lpichler commented Nov 4, 2020

agrare Nov 4, 2020

Choose a reason for hiding this comment

lpichler Nov 5, 2020

Choose a reason for hiding this comment

agrare commented Nov 4, 2020

agrare commented Nov 4, 2020

lpichler commented Nov 5, 2020

agrare commented Nov 5, 2020 • edited Loading

agrare left a comment

Choose a reason for hiding this comment

lpichler commented Nov 5, 2020

lpichler commented Nov 5, 2020

agrare commented Nov 10, 2020 • edited Loading

agrare commented Nov 10, 2020

agrare commented Nov 10, 2020

Fryguy Nov 12, 2020

Choose a reason for hiding this comment

Fryguy Nov 12, 2020

Choose a reason for hiding this comment

lpichler Nov 12, 2020

Choose a reason for hiding this comment

agrare commented Nov 12, 2020

lpichler commented Nov 13, 2020

lpichler commented Nov 13, 2020

agrare Nov 13, 2020

Choose a reason for hiding this comment

lpichler Nov 16, 2020

Choose a reason for hiding this comment

miq-bot commented Nov 16, 2020

lpichler commented Nov 4, 2020 •

edited by agrare

Loading

agrare commented Nov 5, 2020 •

edited

Loading

agrare commented Nov 10, 2020 •

edited

Loading