Skip to content
This repository has been archived by the owner on Mar 22, 2018. It is now read-only.

ContainerManagerService #35

Merged
merged 4 commits into from
Dec 8, 2016

Conversation

ashcrow
Copy link
Collaborator

@ashcrow ashcrow commented Dec 2, 2016

This PR adds the new ContainerManagerService which provides the following remote calls:

  • node_registered: Checks to see if a node has been registered via the ContainerHandler.
  • register_node: Registers a new node using the ContainerHandler.
  • remove_node: Removes a node using the ContainerHandler.
  • get_node_status: Gets the status of the node according to the ContainerHandler.

Requires: projectatomic/commissaire#66 and projectatomic/commissaire#68

Force a retest when the above have both merged.

@ashcrow
Copy link
Collaborator Author

ashcrow commented Dec 2, 2016

@ashcrow ashcrow force-pushed the container-manager-service branch from 922f248 to 4dc7cbc Compare December 5, 2016 16:38
@ashcrow ashcrow force-pushed the container-manager-service branch 3 times, most recently from 315bd9c to 75c5d40 Compare December 7, 2016 16:34
This service owns container management functions such as registering a
node to a container manager. It follows a similar mechanic to the
StorageService in that it is configurable to work with different backend
systems using a handler plugin pattern.
@ashcrow ashcrow force-pushed the container-manager-service branch from 75c5d40 to 465bc67 Compare December 7, 2016 20:16
@ashcrow ashcrow changed the title WIP: ContainerManagerService ContainerManagerService Dec 7, 2016
@ashcrow ashcrow removed the WIP label Dec 7, 2016
@ashcrow
Copy link
Collaborator Author

ashcrow commented Dec 7, 2016

@mbarnes ⬆️

@ashcrow
Copy link
Collaborator Author

ashcrow commented Dec 8, 2016

This will need updating based on the changes in prerequisite PR.

@mbarnes
Copy link
Contributor

mbarnes commented Dec 8, 2016

@ashcrow: As a follow-up, can you also update the Vagrantfile in commissaire to start this new service? (I forgot to do that with clusterexec...)

@ashcrow
Copy link
Collaborator Author

ashcrow commented Dec 8, 2016

@mbarnes will do. I went ahead and put it with the prereq pr so it's ready to go 😄

Copy link
Contributor

@mbarnes mbarnes left a comment

Choose a reason for hiding this comment

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

LGTM! I didn't spot anything that obviously needed changed, but I'll await updates if you have some.

@ashcrow
Copy link
Collaborator Author

ashcrow commented Dec 8, 2016

@mbarnes updated to catch ContainerManagerError and log. Ready for rereview.

Copy link
Contributor

@mbarnes mbarnes left a comment

Choose a reason for hiding this comment

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

Looks good.

@mbarnes mbarnes merged commit 5be6096 into projectatomic:master Dec 8, 2016
@ashcrow ashcrow mentioned this pull request Dec 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants