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.

Sign up for GitHub

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

[2.2] Support for receiving commands to manipulate Rollouts #4040

Merged
merged 22 commits into from
Feb 3, 2022
Merged

[2.2] Support for receiving commands to manipulate Rollouts #4040

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
3c19e3b
Add argo rollouts server dependency [ci-skip]
douglascamata Jan 20, 2022
6f5c7c8
go mod tidy && go mod vendor && make generate
LukeShu Jan 31, 2022
4e53409
Sketch a basic implementation for updating in-cluster rollouts
douglascamata Jan 20, 2022
a31d36a
Add missing actions to RolloutCommand pb
douglascamata Jan 20, 2022
94ad4e9
Handle rollout commands coming over grpc
douglascamata Jan 20, 2022
8b598c2
Improve error handling on rollout commands
douglascamata Jan 20, 2022
969c75d
Add table tests for RolloutCommand
douglascamata Jan 24, 2022
b1c4d8b
Inject the Rollouts Getter creation through the Agent
douglascamata Jan 24, 2022
e2fec9a
Add some comments types in rollouts.go
douglascamata Jan 24, 2022
bf3854b
Add changelog entry about rollout commands
douglascamata Jan 25, 2022
eb1af93
Properly pass the context to run rollout commands
douglascamata Jan 28, 2022
dea4c48
Fix rollout action parsing
douglascamata Jan 31, 2022
1019b2d
Add permission for the agent to patch rollouts
douglascamata Jan 31, 2022
a64e90d
Report results of running rollout commands to the cloud
douglascamata Jan 31, 2022
3ddb8e4
Authenticate command result report call
Feb 1, 2022
03b43da
Fix error log when cannot report command result
Feb 1, 2022
1602547
Update release notes with info about the command reports
Feb 1, 2022
0e97d5e
Merge branch 'master' of github.com:emissary-ingress/emissary into dc…
Feb 2, 2022
fa28a62
Update generated files
Feb 2, 2022
a5a26f0
Fix typo in the changelog
Feb 3, 2022
9c07c67
Merge branch 'master' of github.com:emissary-ingress/emissary into dc…
Feb 3, 2022
c71a957
Update generated manifests
Feb 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const cloudConnectTokenKey = "CLOUD_CONNECT_TOKEN"
type Comm interface {
Close() error
Report(context.Context, *agent.Snapshot, string) error
ReportCommandResult(context.Context, *agent.CommandResult) error
ReportCommandResult(context.Context, *agent.CommandResult, string) error
Directives() <-chan *agent.Directive
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/agent/comm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ func (c *RPCComm) Close() error {
return c.conn.Close()
}

func (c *RPCComm) ReportCommandResult(ctx context.Context, result *agent.CommandResult) error {
func (c *RPCComm) ReportCommandResult(ctx context.Context, result *agent.CommandResult, apiKey string) error {
ctx = metadata.AppendToOutgoingContext(ctx, APIKeyMetadataKey, apiKey)
_, err := c.client.ReportCommandResult(ctx, result, grpc.EmptyCallOption{})
if err != nil {
return fmt.Errorf("ReportCommandResult error: %w", err)
Expand Down
5 changes: 4 additions & 1 deletion pkg/agent/directive_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,10 @@ func (dh *BasicDirectiveHandler) reportCommandResult(ctx context.Context, comman
result.Success = false
result.Message = cmdError.Error()
}
err := a.comm.ReportCommandResult(ctx, result)
a.ambassadorAPIKeyMutex.Lock()
apiKey := a.ambassadorAPIKey
a.ambassadorAPIKeyMutex.Unlock()
Comment on lines +102 to +104
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth a comment that you really don't want to hold this lock during the report-command-result call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kflynn Why? Where would you recommend to use the mutex to access the api key from the agent and send it down to this point?

err := a.comm.ReportCommandResult(ctx, result, apiKey)
if err != nil {
dlog.Errorf(ctx, "error reporting result of rollout command %s: %s", cmd, cmdError)
}
Expand Down