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

Inspect support for exec'ed session #4288

Merged
merged 1 commit into from
Mar 23, 2017
Merged

Inspect support for exec'ed session #4288

merged 1 commit into from
Mar 23, 2017

Conversation

caglar10ur
Copy link
Contributor

Fixes #4264

Includes commits from #4210 for now, will rebase once that gets in (hence WIP).

}

Struct := Type{
Common: CommonPersistence{
ID: "0xDEADBEEF",
Name: "Struct",
},
Notes: "notes",
Copy link
Member

Choose a reason for hiding this comment

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

Had deliberately left this out to ensure that empty fields didn't get serialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will drop this bit

@hickeng
Copy link
Member

hickeng commented Mar 17, 2017

We also need to correct the configuration of runblock so it's not possible if attach is disabled in lib/portlayer/attach/attach.go:toggle

@caglar10ur
Copy link
Contributor Author

I'm planning to fix the runblock in attach PR along with other bits and pieces

@sgairo
Copy link
Contributor

sgairo commented Mar 21, 2017

copyright ranges :) -copyright bot

@caglar10ur caglar10ur changed the title [WIP] Inspect support for exec'ed session Inspect support for exec'ed session Mar 22, 2017
@caglar10ur
Copy link
Contributor Author

ping @cgtexmex @jzt @sflxn

@@ -30,32 +29,24 @@ type VicContainer struct {
HostConfig *containertypes.HostConfig

m sync.RWMutex
execs map[string]*types.ExecConfig
execs map[string]struct{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a dumb question, but where do we keep the ExecConfig objects cached now that we aren't storing them here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming we need to store them, that is. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:) We don't keep ExecConfig because we don't need them. When we create we persist that data on the portlayer side and later when we need them we simply call inspect

Copy link
Member

@dougm dougm left a comment

Choose a reason for hiding this comment

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

lgtm

@cgtexmex
Copy link
Contributor

cgtexmex commented Mar 23, 2017

lgtm

Approved with PullApprove

@caglar10ur caglar10ur merged commit c67ee26 into vmware:master Mar 23, 2017
caglar10ur added a commit that referenced this pull request Apr 27, 2017
* Attach support for exec'ed session

Includes @hmahmood's and @hickeng's various fixed to dio/persona and portlayer

Fixes #4263
Fixes #4367
Fixes #4598
Fixes #4410
Fixes #4339
Fixes #4881

Requires #4288

Implements exec functionality for personality
Those changes live in lib/apiservers/engine/backends/ directory. There are changes in container_proxy functions since exec and attach shares some common code paths. We started to pass ids instead of viccontainer structs around to them to support both workflows. AttachStreams function splits the cancelation logic into different parts. Closing stdin cancels other active streams. We also ensure that the API client connection is shutdown on exit.

Implements exec support for portlayer rest api
Those changes live in lib/apiservers/portlayer/ directory. One important change there is context changes. In the past we were calculating the timeouts manually and then passing timeouts as a parameter to Get call.
session, err := attachServer.Get(context.Background(), params.ID, timeout)

now each stream is creating its own context and passing it to the function - eliminating manual calculations and redundant timeout value.

session, err := i.server.Interaction(ctx, params.ID)

Portlayer attach subsystem re-organization
Those changes live in lib/portlayer/attach/. Some of them just renames and splitting monolithic file (they are now bind.go/join.go etc.) into pieces. Connector has new set of features. We now have a different unblock signal for signaling tether (called unblock) for starting the processes inside the container. We also now have a SSH channel mux running on client side to receive container ids from tether. Normally portlayer initiates the connection and asks container ids. This is still the case for the primary session. But after that primary session start event, any exec'ed session will cause tether to restart and as a part of that tether will push container ids to the portlayer. Additionally we now started to use channel mux to evict closed connections from the connection map (#4367). Lastly we started to use singleflight to make sure that there is only one call in-flight for each ID at any given time.

tether support for exec
As noted above, tether now pushes container ids to portlayer when it receives reload signal. It supports new unblock signal. Fixes a bug caused by wrong usage of cleanup functions. Fixes races causing incorrect behaviors.

dio changes
dio reader now has a PropagateEOF function toggles whether to return EOF when all readers return EOF. Setting this to true will result in an EOF if there are no readers available when Read is next called. dio writer now calls CloseWrite if the writes implements CloseWriter.
matthewavery pushed a commit to matthewavery/vic that referenced this pull request May 3, 2017
* Attach support for exec'ed session

Includes @hmahmood's and @hickeng's various fixed to dio/persona and portlayer

Fixes vmware#4263
Fixes vmware#4367
Fixes vmware#4598
Fixes vmware#4410
Fixes vmware#4339
Fixes vmware#4881

Requires vmware#4288

Implements exec functionality for personality
Those changes live in lib/apiservers/engine/backends/ directory. There are changes in container_proxy functions since exec and attach shares some common code paths. We started to pass ids instead of viccontainer structs around to them to support both workflows. AttachStreams function splits the cancelation logic into different parts. Closing stdin cancels other active streams. We also ensure that the API client connection is shutdown on exit.

Implements exec support for portlayer rest api
Those changes live in lib/apiservers/portlayer/ directory. One important change there is context changes. In the past we were calculating the timeouts manually and then passing timeouts as a parameter to Get call.
session, err := attachServer.Get(context.Background(), params.ID, timeout)

now each stream is creating its own context and passing it to the function - eliminating manual calculations and redundant timeout value.

session, err := i.server.Interaction(ctx, params.ID)

Portlayer attach subsystem re-organization
Those changes live in lib/portlayer/attach/. Some of them just renames and splitting monolithic file (they are now bind.go/join.go etc.) into pieces. Connector has new set of features. We now have a different unblock signal for signaling tether (called unblock) for starting the processes inside the container. We also now have a SSH channel mux running on client side to receive container ids from tether. Normally portlayer initiates the connection and asks container ids. This is still the case for the primary session. But after that primary session start event, any exec'ed session will cause tether to restart and as a part of that tether will push container ids to the portlayer. Additionally we now started to use channel mux to evict closed connections from the connection map (vmware#4367). Lastly we started to use singleflight to make sure that there is only one call in-flight for each ID at any given time.

tether support for exec
As noted above, tether now pushes container ids to portlayer when it receives reload signal. It supports new unblock signal. Fixes a bug caused by wrong usage of cleanup functions. Fixes races causing incorrect behaviors.

dio changes
dio reader now has a PropagateEOF function toggles whether to return EOF when all readers return EOF. Setting this to true will result in an EOF if there are no readers available when Read is next called. dio writer now calls CloseWrite if the writes implements CloseWriter.
fdawg4l pushed a commit to fdawg4l/vic that referenced this pull request May 8, 2017
* Attach support for exec'ed session

Includes @hmahmood's and @hickeng's various fixed to dio/persona and portlayer

Fixes vmware#4263
Fixes vmware#4367
Fixes vmware#4598
Fixes vmware#4410
Fixes vmware#4339
Fixes vmware#4881

Requires vmware#4288

Implements exec functionality for personality
Those changes live in lib/apiservers/engine/backends/ directory. There are changes in container_proxy functions since exec and attach shares some common code paths. We started to pass ids instead of viccontainer structs around to them to support both workflows. AttachStreams function splits the cancelation logic into different parts. Closing stdin cancels other active streams. We also ensure that the API client connection is shutdown on exit.

Implements exec support for portlayer rest api
Those changes live in lib/apiservers/portlayer/ directory. One important change there is context changes. In the past we were calculating the timeouts manually and then passing timeouts as a parameter to Get call.
session, err := attachServer.Get(context.Background(), params.ID, timeout)

now each stream is creating its own context and passing it to the function - eliminating manual calculations and redundant timeout value.

session, err := i.server.Interaction(ctx, params.ID)

Portlayer attach subsystem re-organization
Those changes live in lib/portlayer/attach/. Some of them just renames and splitting monolithic file (they are now bind.go/join.go etc.) into pieces. Connector has new set of features. We now have a different unblock signal for signaling tether (called unblock) for starting the processes inside the container. We also now have a SSH channel mux running on client side to receive container ids from tether. Normally portlayer initiates the connection and asks container ids. This is still the case for the primary session. But after that primary session start event, any exec'ed session will cause tether to restart and as a part of that tether will push container ids to the portlayer. Additionally we now started to use channel mux to evict closed connections from the connection map (vmware#4367). Lastly we started to use singleflight to make sure that there is only one call in-flight for each ID at any given time.

tether support for exec
As noted above, tether now pushes container ids to portlayer when it receives reload signal. It supports new unblock signal. Fixes a bug caused by wrong usage of cleanup functions. Fixes races causing incorrect behaviors.

dio changes
dio reader now has a PropagateEOF function toggles whether to return EOF when all readers return EOF. Setting this to true will result in an EOF if there are no readers available when Read is next called. dio writer now calls CloseWrite if the writes implements CloseWriter.
@caglar10ur caglar10ur deleted the inspect branch May 31, 2017 22:53
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.

7 participants