From 27d728875c2e952b21fe6fa239322bff60cf88e3 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 21 Sep 2020 11:36:38 -0400 Subject: [PATCH] Cleanup session state enum (#394) It's up to the worker to understand what a change in state means, so there was no functional difference between simply sending a state change request and a cancel request. --- .../server_coordination_service.pb.go | 52 ++++++++----------- .../v1/server_coordination_service.proto | 8 ++- .../handlers/workers/worker_service.go | 30 ++++++----- internal/servers/worker/status.go | 3 +- 4 files changed, 44 insertions(+), 49 deletions(-) diff --git a/internal/gen/controller/servers/services/server_coordination_service.pb.go b/internal/gen/controller/servers/services/server_coordination_service.pb.go index b7340611f..f83bd520b 100644 --- a/internal/gen/controller/servers/services/server_coordination_service.pb.go +++ b/internal/gen/controller/servers/services/server_coordination_service.pb.go @@ -188,24 +188,20 @@ type CHANGETYPE int32 const ( CHANGETYPE_CHANGETYPE_UNSPECIFIED CHANGETYPE = 0 - // Indicates that the job should be cancelled on the worker; will also update - // the state on the worker. - CHANGETYPE_CHANGETYPE_CANCEL CHANGETYPE = 1 - // Indicates to the worker to simply update its knowledge of the state. - CHANGETYPE_CHANGETYPE_UPDATE_STATE CHANGETYPE = 2 + // Indicates to the worker to update its knowledge of the state, which may + // cause action to be taken. + CHANGETYPE_CHANGETYPE_UPDATE_STATE CHANGETYPE = 1 ) // Enum value maps for CHANGETYPE. var ( CHANGETYPE_name = map[int32]string{ 0: "CHANGETYPE_UNSPECIFIED", - 1: "CHANGETYPE_CANCEL", - 2: "CHANGETYPE_UPDATE_STATE", + 1: "CHANGETYPE_UPDATE_STATE", } CHANGETYPE_value = map[string]int32{ "CHANGETYPE_UNSPECIFIED": 0, - "CHANGETYPE_CANCEL": 1, - "CHANGETYPE_UPDATE_STATE": 2, + "CHANGETYPE_UPDATE_STATE": 1, } ) @@ -751,27 +747,25 @@ var file_controller_servers_services_v1_server_coordination_service_proto_rawDes 0x4f, 0x42, 0x54, 0x59, 0x50, 0x45, 0x12, 0x17, 0x0a, 0x13, 0x4a, 0x4f, 0x42, 0x54, 0x59, 0x50, 0x45, 0x5f, 0x55, 0x4e, 0x53, 0x50, 0x45, 0x43, 0x49, 0x46, 0x49, 0x45, 0x44, 0x10, 0x00, 0x12, 0x13, 0x0a, 0x0f, 0x4a, 0x4f, 0x42, 0x54, 0x59, 0x50, 0x45, 0x5f, 0x53, 0x45, 0x53, 0x53, 0x49, - 0x4f, 0x4e, 0x10, 0x01, 0x2a, 0x5c, 0x0a, 0x0a, 0x43, 0x48, 0x41, 0x4e, 0x47, 0x45, 0x54, 0x59, + 0x4f, 0x4e, 0x10, 0x01, 0x2a, 0x45, 0x0a, 0x0a, 0x43, 0x48, 0x41, 0x4e, 0x47, 0x45, 0x54, 0x59, 0x50, 0x45, 0x12, 0x1a, 0x0a, 0x16, 0x43, 0x48, 0x41, 0x4e, 0x47, 0x45, 0x54, 0x59, 0x50, 0x45, - 0x5f, 0x55, 0x4e, 0x53, 0x50, 0x45, 0x43, 0x49, 0x46, 0x49, 0x45, 0x44, 0x10, 0x00, 0x12, 0x15, - 0x0a, 0x11, 0x43, 0x48, 0x41, 0x4e, 0x47, 0x45, 0x54, 0x59, 0x50, 0x45, 0x5f, 0x43, 0x41, 0x4e, - 0x43, 0x45, 0x4c, 0x10, 0x01, 0x12, 0x1b, 0x0a, 0x17, 0x43, 0x48, 0x41, 0x4e, 0x47, 0x45, 0x54, - 0x59, 0x50, 0x45, 0x5f, 0x55, 0x50, 0x44, 0x41, 0x54, 0x45, 0x5f, 0x53, 0x54, 0x41, 0x54, 0x45, - 0x10, 0x02, 0x32, 0x86, 0x01, 0x0a, 0x19, 0x53, 0x65, 0x72, 0x76, 0x65, 0x72, 0x43, 0x6f, 0x6f, - 0x72, 0x64, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, - 0x12, 0x69, 0x0a, 0x06, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x2d, 0x2e, 0x63, 0x6f, 0x6e, - 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x2e, - 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x53, 0x74, 0x61, 0x74, - 0x75, 0x73, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x2e, 0x2e, 0x63, 0x6f, 0x6e, 0x74, - 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x2e, 0x73, - 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x53, 0x74, 0x61, 0x74, 0x75, - 0x73, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x42, 0x51, 0x5a, 0x4f, 0x67, - 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, - 0x6f, 0x72, 0x70, 0x2f, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x61, 0x72, 0x79, 0x2f, 0x69, 0x6e, 0x74, - 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, - 0x6c, 0x6c, 0x65, 0x72, 0x2f, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x2f, 0x73, 0x65, 0x72, - 0x76, 0x69, 0x63, 0x65, 0x73, 0x3b, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x62, 0x06, - 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x5f, 0x55, 0x4e, 0x53, 0x50, 0x45, 0x43, 0x49, 0x46, 0x49, 0x45, 0x44, 0x10, 0x00, 0x12, 0x1b, + 0x0a, 0x17, 0x43, 0x48, 0x41, 0x4e, 0x47, 0x45, 0x54, 0x59, 0x50, 0x45, 0x5f, 0x55, 0x50, 0x44, + 0x41, 0x54, 0x45, 0x5f, 0x53, 0x54, 0x41, 0x54, 0x45, 0x10, 0x01, 0x32, 0x86, 0x01, 0x0a, 0x19, + 0x53, 0x65, 0x72, 0x76, 0x65, 0x72, 0x43, 0x6f, 0x6f, 0x72, 0x64, 0x69, 0x6e, 0x61, 0x74, 0x69, + 0x6f, 0x6e, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x69, 0x0a, 0x06, 0x53, 0x74, 0x61, + 0x74, 0x75, 0x73, 0x12, 0x2d, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, + 0x2e, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, + 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x65, 0x71, 0x75, 0x65, + 0x73, 0x74, 0x1a, 0x2e, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, + 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x2e, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, + 0x2e, 0x76, 0x31, 0x2e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, + 0x73, 0x65, 0x22, 0x00, 0x42, 0x51, 0x5a, 0x4f, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, + 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, 0x72, 0x70, 0x2f, 0x62, 0x6f, 0x75, + 0x6e, 0x64, 0x61, 0x72, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x67, + 0x65, 0x6e, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2f, 0x73, 0x65, + 0x72, 0x76, 0x65, 0x72, 0x73, 0x2f, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x3b, 0x73, + 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/internal/proto/local/controller/servers/services/v1/server_coordination_service.proto b/internal/proto/local/controller/servers/services/v1/server_coordination_service.proto index 53e730135..0c5cc94c0 100644 --- a/internal/proto/local/controller/servers/services/v1/server_coordination_service.proto +++ b/internal/proto/local/controller/servers/services/v1/server_coordination_service.proto @@ -69,11 +69,9 @@ message StatusRequest { enum CHANGETYPE { CHANGETYPE_UNSPECIFIED = 0; - // Indicates that the job should be cancelled on the worker; will also update - // the state on the worker. - CHANGETYPE_CANCEL = 1; - // Indicates to the worker to simply update its knowledge of the state. - CHANGETYPE_UPDATE_STATE = 2; + // Indicates to the worker to update its knowledge of the state, which may + // cause action to be taken. + CHANGETYPE_UPDATE_STATE = 1; } message JobChangeRequest { diff --git a/internal/servers/controller/handlers/workers/worker_service.go b/internal/servers/controller/handlers/workers/worker_service.go index 408c0ae48..263f51757 100644 --- a/internal/servers/controller/handlers/workers/worker_service.go +++ b/internal/servers/controller/handlers/workers/worker_service.go @@ -100,21 +100,25 @@ func (ws *workerServiceServer) Status(ctx context.Context, req *pbs.StatusReques // it's in termianted status something went wrong and we're // mismatched, so ensure we cancel it also. currState := sessionInfo.States[0].Status - switch currState { - case session.StatusCancelling, - session.StatusTerminated: - ret.JobsRequests = append(ret.JobsRequests, &pbs.JobChangeRequest{ - Job: &pbs.Job{ - Type: pbs.JOBTYPE_JOBTYPE_SESSION, - JobInfo: &pbs.Job_SessionInfo{ - SessionInfo: &pbs.SessionJobInfo{ - SessionId: sessionId, - Status: currState.ProtoVal(), + if currState.ProtoVal() != si.Status { + switch currState { + case session.StatusCancelling, + session.StatusTerminated: + // If we're here the job is pending or active so we do want + // to actually send a change request + ret.JobsRequests = append(ret.JobsRequests, &pbs.JobChangeRequest{ + Job: &pbs.Job{ + Type: pbs.JOBTYPE_JOBTYPE_SESSION, + JobInfo: &pbs.Job_SessionInfo{ + SessionInfo: &pbs.SessionJobInfo{ + SessionId: sessionId, + Status: currState.ProtoVal(), + }, }, }, - }, - RequestType: pbs.CHANGETYPE_CHANGETYPE_CANCEL, - }) + RequestType: pbs.CHANGETYPE_CHANGETYPE_UPDATE_STATE, + }) + } } } } diff --git a/internal/servers/worker/status.go b/internal/servers/worker/status.go index 3faee341a..d625bd76b 100644 --- a/internal/servers/worker/status.go +++ b/internal/servers/worker/status.go @@ -109,8 +109,7 @@ func (w *Worker) startStatusTicking(cancelCtx context.Context) { for _, request := range result.GetJobsRequests() { switch request.GetRequestType() { - case pbs.CHANGETYPE_CHANGETYPE_UPDATE_STATE, - pbs.CHANGETYPE_CHANGETYPE_CANCEL: + case pbs.CHANGETYPE_CHANGETYPE_UPDATE_STATE: switch request.GetJob().GetType() { case pbs.JOBTYPE_JOBTYPE_SESSION: sessInfo := request.GetJob().GetSessionInfo()