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

Add controls form container start, stop, pause, etc #598

Merged
merged 3 commits into from
Nov 6, 2015

Conversation

tomwilkie
Copy link
Contributor

Add control interface for Scope. Allows UI to do RPCs all the way back to the probe, with the app doing the routing in the middle. Add metadata in the API so these controls can be appropriately rendered, and add some basic support in the UI.

This is a very hacky, very early implementation. Fixes #315 (see bug for some design notes).

Also:

  • Docker integration adds controls for container lifecycle
  • include non-running docker containers in the report, dynamically update docker container states
  • add a filter to filter out non-running containers

Todo

  • Make it lint & pass tests
  • Try and use bits and bobs from golang rpc
  • Separate control http mux/router in the app
  • Add tests for capabilities
  • Reorganise change sets.
  • Make mutli client delete old apps
  • Document / comment code
  • Integration test
  • Make UI reflect result of control RPC (@davkal?)

Punted out of this PR:

@tomwilkie tomwilkie self-assigned this Oct 27, 2015
@@ -11,6 +12,15 @@ const NodeDetailsTable = React.createClass({
{this.props.title}
</h4>


<div className="node-details-table-row">
{this.props.capabilities.map(function(capability) {

This comment was marked as abuse.

This comment was marked as abuse.

@tomwilkie tomwilkie changed the title [WIP] Beginnings of capability support. [WIP] Add controls Oct 28, 2015
handlers = map[string]xfer.CapabilityHandler{}
)

// HandleCapabilityRequest performs a capability request.

This comment was marked as abuse.

@paulbellamy
Copy link
Contributor

Just curious how you see the request/response model working with longer-running (interactive) capabilities, like shell? Bridging through a websocket to the user's browser? or separate request/response for each chunk of data?

const capabilityKey = "capability"

// CapabilityRouter handles the impedence mismatch between posted Capability calls,
// and websocket connections.

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor Author

Just curious how you see the request/response model working with longer-running (interactive) capabilities, like shell?

I don't think we can build shell / logs with this just yet. I think we need to layer some more abstraction/plumbing on top first.

I'm currently thinking that a request could return a 'pipe' instead a response. The pipe would have a different lifecycle, and use different messages on the same websocket. We might be able to use app<->client websocket too.

Anyway I suggest we get the short-lived RPCs bit (start, stop, restart etc) right first, then think about logs & terminals.

@tomwilkie tomwilkie force-pushed the 315-capabilities branch 6 times, most recently from 260c822 to 9e71d98 Compare October 30, 2015 10:03
nodeID = vars["nodeID"]
control = vars["control"]
)
handler, ok := cr.probes[probeID]

This comment was marked as abuse.

This comment was marked as abuse.

@paulbellamy
Copy link
Contributor

Would it make sense to have this? i.e. treating controls as a "plugin" with app and probe components.

.
└── controls
    ├── app
        └── *.go
    └── probe
        └── *.go

Thinking about it, they are fairly tightly integrated with the docker probe plugin and everything else, so maybe this doesn't make sense...

@tomwilkie
Copy link
Contributor Author

re: treating as a plugin; as you say, the code it pretty tightly integrated, so I think the division you suggest would be pretty artificial.

I'm trying to do the following:

  • have the common bits (ie on the wire format, codecs etc) in xfer/
  • have the app specific bits (accepting a control from the UI, routing it to the right probe) in the app/
  • have the probe specific bits (handling the control, routing to appropriate handler) in probe/

The bit that doesn't fit it the probe's connection to the app logic, which I've put in xfer/, as thats where the http publisher are. On could argue this belongs in probe/, but I'm not too fussed either way.

return xfer.ResponseErrorf("Invalid ID: %s", req.NodeID)
}

return xfer.ResponseError(container.Stop())

This comment was marked as abuse.

This comment was marked as abuse.

@paulbellamy
Copy link
Contributor

There's a period when stopping or starting the container that you get all three controls available.
screen shot 2015-11-02 at 13 13 47

Also, the controls take quite a while to have a visible effect, and there's very little feedback that it is doing something.

@paulbellamy
Copy link
Contributor

Should probably show the container state in the details panel.

@paulbellamy
Copy link
Contributor

Needs a make static, also.

@tomwilkie
Copy link
Contributor Author

Should probably show the container state in the details panel.

done.

@tomwilkie
Copy link
Contributor Author

There's a period when stopping or starting the container that you get all three controls available.

After discussion, this has been punted to the next PR.

@tomwilkie tomwilkie force-pushed the 315-capabilities branch 6 times, most recently from 38da875 to 970f75f Compare November 3, 2015 12:02
@tomwilkie tomwilkie changed the title [WIP] Add controls Add controls Nov 3, 2015
@tomwilkie tomwilkie changed the title Add controls Add controls form container start, stop, pause, etc Nov 3, 2015
@tomwilkie tomwilkie removed their assignment Nov 3, 2015
)

func TestControls(t *testing.T) {
controls.Register("foo", func(req xfer.Request) xfer.Response {

This comment was marked as abuse.

This comment was marked as abuse.

@tomwilkie tomwilkie force-pushed the 315-capabilities branch 2 times, most recently from 7cc0fca to a8edc2c Compare November 4, 2015 16:38
@tomwilkie
Copy link
Contributor Author

+0.2% on coverage with 1.2k new loc!

@davkal davkal self-assigned this Nov 5, 2015
@paulbellamy
Copy link
Contributor

Backend LGTM

…g, and a filter for those containers.

Also add integration test for container controls.
- Make backend address configurable via env variable
- `BACKEND_HOST=1.2.3.4:4040 npm start` points the frontend to the app on that server. Just for development
- Render control icons
  - removed close x for details panel
  - added control icons in its space
  - closing of panel still works by clicking on same node, or background
- Dont allow control while pending
- Render and auto-dismiss control error
- Make tests pass
@tomwilkie
Copy link
Contributor Author

Frontend LGTM.

tomwilkie added a commit that referenced this pull request Nov 6, 2015
Add controls form container start, stop, pause, etc
@tomwilkie tomwilkie merged commit 39b6630 into master Nov 6, 2015
@tomwilkie tomwilkie deleted the 315-capabilities branch November 6, 2015 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants