From 82a9595a6a35c648fff62c2497d858f1ab062578 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Tue, 26 Jan 2021 14:53:16 -0500 Subject: [PATCH] 17170: Add interactive_session_started flag to containers. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- .../methods/containers.html.textile.liquid | 2 + lib/controller/localdb/container_gateway.go | 16 ++++++- .../localdb/container_gateway_test.go | 5 ++ sdk/go/arvados/container.go | 47 ++++++++++--------- services/api/app/models/container.rb | 4 ++ ...teractive_session_started_to_containers.rb | 9 ++++ services/api/db/structure.sql | 6 ++- services/api/test/unit/container_test.rb | 2 + 8 files changed, 65 insertions(+), 26 deletions(-) create mode 100644 services/api/db/migrate/20210126183521_add_interactive_session_started_to_containers.rb diff --git a/doc/api/methods/containers.html.textile.liquid b/doc/api/methods/containers.html.textile.liquid index 8a7ebc36e5..096a1fcaa9 100644 --- a/doc/api/methods/containers.html.textile.liquid +++ b/doc/api/methods/containers.html.textile.liquid @@ -57,6 +57,8 @@ Generally this will contain additional keys that are not present in any correspo |auth_uuid|string|UUID of a token to be passed into the container itself, used to access Keep-backed mounts, etc. Automatically assigned.|Null if state∉{"Locked","Running"} or if @runtime_token@ was provided.| |locked_by_uuid|string|UUID of a token, indicating which dispatch process changed state to Locked. If null, any token can be used to lock. If not null, only the indicated token can modify this container.|Null if state∉{"Locked","Running"}| |runtime_token|string|A v2 token to be passed into the container itself, used to access Keep-backed mounts, etc.|Not returned in API responses. Reset to null when state is "Complete" or "Cancelled".| +|gateway_address|string|Address (host:port) of gateway server.|Internal use only.| +|interactive_session_started|boolean|Indicates whether @arvados-client shell@ has been used to run commands in the container, which may have altered the container's behavior and output.|| h2(#container_states). Container states diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go index b8d85516a2..e509278773 100644 --- a/lib/controller/localdb/container_gateway.go +++ b/lib/controller/localdb/container_gateway.go @@ -38,12 +38,12 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt if err != nil { return } + ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}}) if !user.IsAdmin || !conn.cluster.Containers.ShellAccess.Admin { if !conn.cluster.Containers.ShellAccess.User { err = httpserver.ErrorWithStatus(errors.New("shell access is disabled in config"), http.StatusServiceUnavailable) return } - ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}}) var crs arvados.ContainerRequestList crs, err = conn.railsProxy.ContainerRequestList(ctxRoot, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"container_uuid", "=", opts.UUID}}}) if err != nil { @@ -153,6 +153,20 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt netconn.Close() return } + + if !ctr.InteractiveSessionStarted { + _, err = conn.railsProxy.ContainerUpdate(ctxRoot, arvados.UpdateOptions{ + UUID: opts.UUID, + Attrs: map[string]interface{}{ + "interactive_session_started": true, + }, + }) + if err != nil { + netconn.Close() + return + } + } + sshconn.Conn = netconn sshconn.Bufrw = &bufio.ReadWriter{Reader: bufr, Writer: bufw} sshconn.Logger = ctxlog.FromContext(ctx) diff --git a/lib/controller/localdb/container_gateway_test.go b/lib/controller/localdb/container_gateway_test.go index 54c1b2149a..aff569b098 100644 --- a/lib/controller/localdb/container_gateway_test.go +++ b/lib/controller/localdb/container_gateway_test.go @@ -77,6 +77,8 @@ func (s *ContainerGatewaySuite) SetUpSuite(c *check.C) { func (s *ContainerGatewaySuite) SetUpTest(c *check.C) { s.cluster.Containers.ShellAccess.Admin = true s.cluster.Containers.ShellAccess.User = true + _, err := arvadostest.DB(c, s.cluster).Exec(`update containers set interactive_session_started=$1 where uuid=$2`, false, s.ctrUUID) + c.Check(err, check.IsNil) } func (s *ContainerGatewaySuite) TestConfig(c *check.C) { @@ -152,6 +154,9 @@ func (s *ContainerGatewaySuite) TestConnect(c *check.C) { case <-time.After(time.Second): c.Fail() } + ctr, err := s.localdb.ContainerGet(s.ctx, arvados.GetOptions{UUID: s.ctrUUID}) + c.Check(err, check.IsNil) + c.Check(ctr.InteractiveSessionStarted, check.Equals, true) } func (s *ContainerGatewaySuite) TestConnectFail(c *check.C) { diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go index 4d1511b412..86b7c86846 100644 --- a/sdk/go/arvados/container.go +++ b/sdk/go/arvados/container.go @@ -8,29 +8,30 @@ import "time" // Container is an arvados#container resource. type Container struct { - UUID string `json:"uuid"` - Etag string `json:"etag"` - CreatedAt time.Time `json:"created_at"` - ModifiedByClientUUID string `json:"modified_by_client_uuid"` - ModifiedByUserUUID string `json:"modified_by_user_uuid"` - ModifiedAt time.Time `json:"modified_at"` - Command []string `json:"command"` - ContainerImage string `json:"container_image"` - Cwd string `json:"cwd"` - Environment map[string]string `json:"environment"` - LockedByUUID string `json:"locked_by_uuid"` - Mounts map[string]Mount `json:"mounts"` - Output string `json:"output"` - OutputPath string `json:"output_path"` - Priority int64 `json:"priority"` - RuntimeConstraints RuntimeConstraints `json:"runtime_constraints"` - State ContainerState `json:"state"` - SchedulingParameters SchedulingParameters `json:"scheduling_parameters"` - ExitCode int `json:"exit_code"` - RuntimeStatus map[string]interface{} `json:"runtime_status"` - StartedAt *time.Time `json:"started_at"` // nil if not yet started - FinishedAt *time.Time `json:"finished_at"` // nil if not yet finished - GatewayAddress string `json:"gateway_address"` + UUID string `json:"uuid"` + Etag string `json:"etag"` + CreatedAt time.Time `json:"created_at"` + ModifiedByClientUUID string `json:"modified_by_client_uuid"` + ModifiedByUserUUID string `json:"modified_by_user_uuid"` + ModifiedAt time.Time `json:"modified_at"` + Command []string `json:"command"` + ContainerImage string `json:"container_image"` + Cwd string `json:"cwd"` + Environment map[string]string `json:"environment"` + LockedByUUID string `json:"locked_by_uuid"` + Mounts map[string]Mount `json:"mounts"` + Output string `json:"output"` + OutputPath string `json:"output_path"` + Priority int64 `json:"priority"` + RuntimeConstraints RuntimeConstraints `json:"runtime_constraints"` + State ContainerState `json:"state"` + SchedulingParameters SchedulingParameters `json:"scheduling_parameters"` + ExitCode int `json:"exit_code"` + RuntimeStatus map[string]interface{} `json:"runtime_status"` + StartedAt *time.Time `json:"started_at"` // nil if not yet started + FinishedAt *time.Time `json:"finished_at"` // nil if not yet finished + GatewayAddress string `json:"gateway_address"` + InteractiveSessionStarted bool `json:"interactive_session_started"` } // ContainerRequest is an arvados#container_request resource. diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index 52fc79e2c0..49be3df558 100644 --- a/services/api/app/models/container.rb +++ b/services/api/app/models/container.rb @@ -77,6 +77,7 @@ class Container < ArvadosModel t.add :runtime_auth_scopes t.add :lock_count t.add :gateway_address + t.add :interactive_session_started end # Supported states for a container @@ -481,6 +482,9 @@ class Container < ArvadosModel if self.state_changed? permitted.push :started_at, :gateway_address end + if !self.interactive_session_started_was + permitted.push :interactive_session_started + end when Complete if self.state_was == Running diff --git a/services/api/db/migrate/20210126183521_add_interactive_session_started_to_containers.rb b/services/api/db/migrate/20210126183521_add_interactive_session_started_to_containers.rb new file mode 100644 index 0000000000..3fe23f64d8 --- /dev/null +++ b/services/api/db/migrate/20210126183521_add_interactive_session_started_to_containers.rb @@ -0,0 +1,9 @@ +# Copyright (C) The Arvados Authors. All rights reserved. +# +# SPDX-License-Identifier: AGPL-3.0 + +class AddInteractiveSessionStartedToContainers < ActiveRecord::Migration[5.2] + def change + add_column :containers, :interactive_session_started, :boolean, null: false, default: false + end +end diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql index 249ec67ac3..14eca609eb 100644 --- a/services/api/db/structure.sql +++ b/services/api/db/structure.sql @@ -522,7 +522,8 @@ CREATE TABLE public.containers ( runtime_auth_scopes jsonb, runtime_token text, lock_count integer DEFAULT 0 NOT NULL, - gateway_address character varying + gateway_address character varying, + interactive_session_started boolean DEFAULT false NOT NULL ); @@ -3189,6 +3190,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20201103170213'), ('20201105190435'), ('20201202174753'), -('20210108033940'); +('20210108033940'), +('20210126183521'); diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb index 98e60e0579..7853b6f6a9 100644 --- a/services/api/test/unit/container_test.rb +++ b/services/api/test/unit/container_test.rb @@ -797,6 +797,8 @@ class ContainerTest < ActiveSupport::TestCase [Container::Running, {priority: 123456789}], [Container::Running, {runtime_status: {'error' => 'oops'}}], [Container::Running, {cwd: '/'}], + [Container::Running, {gateway_address: "172.16.0.1:12345"}], + [Container::Running, {interactive_session_started: true}], [Container::Complete, {state: Container::Cancelled}], [Container::Complete, {priority: 123456789}], [Container::Complete, {runtime_status: {'error' => 'oops'}}], -- 2.30.2