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

Added container components for service model. #12863

Merged
merged 1 commit into from
Jan 16, 2017
Merged

Added container components for service model. #12863

merged 1 commit into from
Jan 16, 2017

Conversation

kevensen
Copy link
Contributor

OpenShift (and theoretically other container platforms) objects should be treated as first class citizens in the Automate Service Model in ManageIQ. This will enable customized workflows using these objects.

This pull request only adds code that allows one to Read the objects. Future work will be made to allow an automate workflow to Create, Update, and Delete the objects.

https://bugzilla.redhat.com/show_bug.cgi?id=1378190

@@ -0,0 +1,4 @@
module MiqAeMethodService
class MiqAeServiceContainer < MiqAeServiceModelBase
Copy link
Contributor

Choose a reason for hiding this comment

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

@kevensen
From the service model are you planning on exposing its relationships or just the attributes.
With the current implementation only the attributes are going to be exposed and not any relationships.
https://github.com/ManageIQ/manageiq/blob/master/lib/miq_automation_engine/service_models/miq_ae_service_host.rb#L6 shows how we expose relationships

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mkanoor I want to expose some of the relationships. Thank you for the tip. I'll update the pull request in the next day or two. Or would you rather me close this request and open a new one?

Copy link
Contributor

Choose a reason for hiding this comment

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

@kevensen
You can just add new commits to this PR

@mkanoor
Copy link
Contributor

mkanoor commented Dec 2, 2016

@kevensen
Also please add specs for the service model expose associations. This ensures that we only expose the associations that are valid.
https://github.com/ManageIQ/manageiq/blob/master/spec/lib/miq_automation_engine/service_methods/miq_ae_service_user_spec.rb
has a sample

@kevensen
Copy link
Contributor Author

kevensen commented Dec 4, 2016

@mkanoor - I've looked at the specs for the service model exposed associations. Please forgive me that my Ruby is a little weak. Are these specs checked at runtime? Or are these like unit tests?

@mkanoor
Copy link
Contributor

mkanoor commented Dec 5, 2016

@kevensen
They are like unit tests, these just validate that the required functions are exposed in case someone later tries to remove one of the exposed methods.

@kevensen
Copy link
Contributor Author

kevensen commented Dec 8, 2016

@mkanoor - I've completed the service model, associations, and specs. At your convenience please review. I look forward to and appreciate any feedback you have.

@@ -1,3 +1,9 @@
class ContainerComponentStatus < ApplicationRecord
belongs_to :ext_management_system, :foreign_key => "ems_id"

virtual_column :name, :type => :string
Copy link
Contributor

Choose a reason for hiding this comment

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

@blomquisg
Hi Greg can you have someone from provider team review the app/models changes.

@mkanoor
Copy link
Contributor

mkanoor commented Jan 13, 2017

@kevensen
Can you please move the app/model changes do a different PR since this PR is just about service models.

@miq-bot
Copy link
Member

miq-bot commented Jan 14, 2017

Checked commit kevensen@78cfe13 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
45 files checked, 0 offenses detected
Everything looks good. 👍

@kevensen
Copy link
Contributor Author

@mkanoor Certainly. I've reverted those files back.

@mkanoor mkanoor merged commit 3650d79 into ManageIQ:master Jan 16, 2017
@mkanoor mkanoor added this to the Sprint 52 Ending Jan 16, 2017 milestone Jan 16, 2017
@gmcculloug
Copy link
Member

@kevensen Is this valid for back-port to the Euwe branch? Please label euwe/yes if so. If this cannot be back-ported let's discuss and determine what needs to be pulled back to fix https://bugzilla.redhat.com/show_bug.cgi?id=1437128.

@kevensen
Copy link
Contributor Author

kevensen commented Apr 4, 2017

@gmcculloug I just ran the test suite against these components in the euwe branch and it looked fine. So, yes, I believe this is valid for a back-port. I'd be happy to change the label, but not being a member I don't think I have the permission to do so.

@gmcculloug
Copy link
Member

@kevensen I set the flag but it is also available for others through the miq-bot. See documentation here: https://github.com/ManageIQ/miq_bot

@moolitayer
Copy link

Still waiting for bz creation but we asked https://bugzilla.redhat.com/show_bug.cgi?id=1437128 marked as a blocker for euwe.

@miq-bot add_label blocker

@miq-bot miq-bot added the blocker label Apr 4, 2017
@moolitayer
Copy link

@gmcculloug I'm looking into unit testing the automate part in container SSA (check policy prevent). Do you have any pointers (other tests that run an automate worker?)

simaishi pushed a commit that referenced this pull request Apr 4, 2017
@simaishi
Copy link
Contributor

simaishi commented Apr 4, 2017

Euwe backport details:

$ git log -1
commit dfbb9d9d2ac46578cf9273fa398da5da8f408bf3
Author: Madhu Kanoor <mkanoor@redhat.com>
Date:   Mon Jan 16 10:25:26 2017 -0500

    Merge pull request #12863 from kevensen/service_model_containers
    
    Added container components for service model.
    (cherry picked from commit 3650d79e226f6501c397dbc9bf6e785d7561bbfb)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1437128

@pemcg
Copy link

pemcg commented May 25, 2017

It would be great if these service models could support custom attributes so that we could label the objects from automate

@gmcculloug
Copy link
Member

@kevensen
Copy link
Contributor Author

@pemcg @gmcculloug For a frame of reference, can you please remind me what would be a custom attribute for a VM? I think it would be a good idea to tie these to labels on container resources.

@pemcg
Copy link

pemcg commented May 25, 2017

Custom attributes allow users to add their own key/value text pairs to objects. A typical example might be a Configuration ID from a CMDB, or a 'status' text string. It's useful to be able to define them from automate using the custom_set method even if they are not necessarily visible in the WebUI.

This chapter https://pemcg.gitbooks.io/mastering-automation-in-cloudforms-4-2-and-manage/content/working_with_virtual_machines/chapter.html describes how to set custom attributes on a VM if it helps.

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.

8 participants