Skip to content

Commit

Permalink
fix: replaced deprecated references, types.AuthConfig and dcontainer.…
Browse files Browse the repository at this point in the history
…ContainerWaitOKBody*
  • Loading branch information
corban-beaird authored and maxrussell committed Dec 9, 2023
1 parent b332dd8 commit 841d2ab
Show file tree
Hide file tree
Showing 14 changed files with 71 additions and 69 deletions.
17 changes: 9 additions & 8 deletions agent/pkg/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/docker/docker/api/types"
dcontainer "github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/filters"
typeReg "github.com/docker/docker/api/types/registry"
"github.com/docker/docker/client"
"github.com/docker/docker/pkg/jsonmessage"
"github.com/docker/docker/registry"
Expand Down Expand Up @@ -60,7 +61,7 @@ type (
// Results on the Waiter channel indicate changes in container state, while results on the
// Errs channel indicate failures to watch for updates.
ContainerWaiter struct {
Waiter <-chan dcontainer.ContainerWaitOKBody
Waiter <-chan dcontainer.WaitResponse
Errs <-chan error
}
// Container contains details about a running container and waiters to await its termination.
Expand All @@ -74,7 +75,7 @@ type (
type Client struct {
// Configuration details. Set during initialization, never modified afterwards.
credentialStores map[string]*credentialStore
authConfigs map[string]types.AuthConfig
authConfigs map[string]typeReg.AuthConfig

// System dependencies. Also set during initialization, never modified afterwards.
cl *client.Client
Expand Down Expand Up @@ -157,7 +158,7 @@ func (d *Client) ReattachContainer(
type PullImage struct {
Name string
ForcePull bool
Registry *types.AuthConfig
Registry *typeReg.AuthConfig
}

// PullImage pulls an image according to the given request and credentials initialized at client
Expand Down Expand Up @@ -363,9 +364,9 @@ func LabelFilter(key, val string) filters.Args {
func (d *Client) getDockerAuths(
ctx context.Context,
image reference.Named,
userRegistry *types.AuthConfig,
userRegistry *typeReg.AuthConfig,
p events.Publisher[Event],
) (*types.AuthConfig, error) {
) (*typeReg.AuthConfig, error) {
imageDomain := reference.Domain(image)
// Try user submitted registry auth config.
if userRegistry != nil {
Expand Down Expand Up @@ -415,7 +416,7 @@ func (d *Client) getDockerAuths(
return nil, fmt.Errorf("error invalid docker repo name: %w", err)
}
reg := registry.ResolveAuthConfig(d.authConfigs, index)
if reg == (types.AuthConfig{}) {
if reg == (typeReg.AuthConfig{}) {
return &reg, nil
}

Expand All @@ -428,8 +429,8 @@ func (d *Client) getDockerAuths(
}

// registryToString converts the Registry struct to a base64 encoding for json strings.
func registryToString(reg types.AuthConfig) (string, error) {
// Docker stores the username and password in an auth section types.AuthConfig
func registryToString(reg typeReg.AuthConfig) (string, error) {
// Docker stores the username and password in an auth section typeReg.AuthConfig
// formatted as user:pass then base64ed. This is not documented clearly.
// https://github.com/docker/cli/blob/master/cli/config/configfile/file.go#L76
if reg.Auth != "" {
Expand Down
12 changes: 6 additions & 6 deletions agent/pkg/docker/docker_credential_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"path"

hclient "github.com/docker/docker-credential-helpers/client"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/registry"
"github.com/pkg/errors"
)

Expand All @@ -31,7 +31,7 @@ func getDockerConfigPath() (string, error) {

// processDockerConfig reads a user's ~/.docker/config.json and returns
// credential helpers configured and the "auths" section of the config.
func processDockerConfig() (map[string]*credentialStore, map[string]types.AuthConfig, error) {
func processDockerConfig() (map[string]*credentialStore, map[string]registry.AuthConfig, error) {
dockerConfigFile, err := getDockerConfigPath()
if err != nil {
return nil, nil, err
Expand All @@ -43,8 +43,8 @@ func processDockerConfig() (map[string]*credentialStore, map[string]types.AuthCo
}

var config struct {
CredentialHelpers map[string]string `json:"credHelpers"`
Auths map[string]types.AuthConfig `json:"auths"`
CredentialHelpers map[string]string `json:"credHelpers"`
Auths map[string]registry.AuthConfig `json:"auths"`
}
if err := json.Unmarshal(b, &config); err != nil {
return nil, nil, errors.Wrap(err, "can't parse docker config")
Expand All @@ -62,8 +62,8 @@ func processDockerConfig() (map[string]*credentialStore, map[string]types.AuthCo
}

// get executes the command to get the credentials from the native store.
func (s *credentialStore) get() (types.AuthConfig, error) {
var ret types.AuthConfig
func (s *credentialStore) get() (registry.AuthConfig, error) {
var ret registry.AuthConfig

creds, err := hclient.Get(s.store, s.registry)
if err != nil {
Expand Down
26 changes: 13 additions & 13 deletions agent/pkg/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"testing"

"github.com/docker/distribution/reference"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/registry"

"github.com/determined-ai/determined/agent/pkg/events"

Expand All @@ -18,23 +18,23 @@ func TestGetDockerAuths(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

dockerhubAuthConfig := types.AuthConfig{
dockerhubAuthConfig := registry.AuthConfig{
Username: "username",
Password: "password",
ServerAddress: "docker.io",
}

exampleDockerConfig := types.AuthConfig{
exampleDockerConfig := registry.AuthConfig{
Auth: "token",
ServerAddress: "https://example.com",
}

noServerAuthConfig := types.AuthConfig{
noServerAuthConfig := registry.AuthConfig{
Username: "username",
Password: "password",
}

dockerAuthSection := map[string]types.AuthConfig{
dockerAuthSection := map[string]registry.AuthConfig{
"https://index.docker.io/v1/": {
Auth: "dockerhubtoken",
ServerAddress: "docker.io",
Expand All @@ -47,18 +47,18 @@ func TestGetDockerAuths(t *testing.T) {

cases := []struct {
image string
expconfReg *types.AuthConfig
authConfigs map[string]types.AuthConfig
expected types.AuthConfig
expconfReg *registry.AuthConfig
authConfigs map[string]registry.AuthConfig
expected registry.AuthConfig
}{
// No authentication passed in.
{"detai", nil, nil, types.AuthConfig{}},
{"detai", nil, nil, registry.AuthConfig{}},
// Correct server passed in for dockerhub.
{"detai", &dockerhubAuthConfig, nil, dockerhubAuthConfig},
// Correct server passed in for example.com.
{"example.com/detai", &exampleDockerConfig, nil, exampleDockerConfig},
// Different server passed than specified auth.
{"example.com/detai", &dockerhubAuthConfig, nil, types.AuthConfig{}},
{"example.com/detai", &dockerhubAuthConfig, nil, registry.AuthConfig{}},
// No server (behavior is deprecated).
{"detai", &noServerAuthConfig, nil, noServerAuthConfig},
{"example.com/detai", &noServerAuthConfig, nil, noServerAuthConfig},
Expand All @@ -73,7 +73,7 @@ func TestGetDockerAuths(t *testing.T) {
dockerAuthSection["example.com"],
},
// We don't return a result if we don't have that serveraddress.
{"determined.ai/detai", nil, dockerAuthSection, types.AuthConfig{}},
{"determined.ai/detai", nil, dockerAuthSection, registry.AuthConfig{}},
}

evs := make(chan Event, 100)
Expand All @@ -95,7 +95,7 @@ func TestGetDockerAuths(t *testing.T) {

func TestRegistryToString(t *testing.T) {
// No auth just base64ed.
case1 := types.AuthConfig{
case1 := registry.AuthConfig{
Email: "[email protected]",
Password: "password",
}
Expand All @@ -108,7 +108,7 @@ func TestRegistryToString(t *testing.T) {
// Auth gets split.
user, pass := "user", "pass"
auth := fmt.Sprintf("%s:%s", user, pass)
case2 := types.AuthConfig{
case2 := registry.AuthConfig{
Auth: base64.StdEncoding.EncodeToString([]byte(auth)),
}
expected = base64.URLEncoding.EncodeToString([]byte(fmt.Sprintf(
Expand Down
6 changes: 3 additions & 3 deletions agent/pkg/podman/podman.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,20 +408,20 @@ func (s *PodmanClient) waitOnContainer(
cont *PodmanContainer,
p events.Publisher[docker.Event],
) docker.ContainerWaiter {
wchan := make(chan dcontainer.ContainerWaitOKBody, 1)
wchan := make(chan dcontainer.WaitResponse, 1)
errchan := make(chan error)
s.wg.Go(func(ctx context.Context) {
defer close(wchan)
defer close(errchan)

var body dcontainer.ContainerWaitOKBody
var body dcontainer.WaitResponse
switch state, err := cont.Proc.Wait(); {
case ctx.Err() != nil && err == nil && state.ExitCode() == -1:
s.log.Trace("detached from container process")
return
case err != nil:
s.log.Tracef("proc %d for container %s exited: %s", cont.PID, id, err)
body.Error = &dcontainer.ContainerWaitOKBodyError{Message: err.Error()}
body.Error = &dcontainer.WaitExitError{Message: err.Error()}
default:
s.log.Tracef("proc %d for container %s exited with %d", cont.PID, id, state.ExitCode())
body.StatusCode = int64(state.ExitCode())
Expand Down
6 changes: 3 additions & 3 deletions agent/pkg/singularity/singularity.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,20 +543,20 @@ func (s *SingularityClient) waitOnContainer(
cont *SingularityContainer,
p events.Publisher[docker.Event],
) docker.ContainerWaiter {
wchan := make(chan dcontainer.ContainerWaitOKBody, 1)
wchan := make(chan dcontainer.WaitResponse, 1)
errchan := make(chan error)
s.wg.Go(func(ctx context.Context) {
defer close(wchan)
defer close(errchan)

var body dcontainer.ContainerWaitOKBody
var body dcontainer.WaitResponse
switch state, err := cont.Proc.Wait(); {
case ctx.Err() != nil && err == nil && state.ExitCode() == -1:
s.log.Trace("detached from container process")
return
case err != nil:
s.log.Tracef("proc %d for container %s exited: %s", cont.PID, id, err)
body.Error = &dcontainer.ContainerWaitOKBodyError{Message: err.Error()}
body.Error = &dcontainer.WaitExitError{Message: err.Error()}
default:
s.log.Tracef("proc %d for container %s exited with %d", cont.PID, id, state.ExitCode())
body.StatusCode = int64(state.ExitCode())
Expand Down
4 changes: 2 additions & 2 deletions agent/pkg/singularity/singularity_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import (
"os/user"
"testing"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/registry"
"github.com/docker/docker/api/types/strslice"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -40,7 +40,7 @@ func TestSingularity(t *testing.T) {
pub := events.ChannelPublisher(evs)
err = cl.PullImage(ctx, docker.PullImage{
Name: image,
Registry: &types.AuthConfig{},
Registry: &registry.AuthConfig{},
}, pub)
require.NoError(t, err)

Expand Down
4 changes: 2 additions & 2 deletions master/internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"testing"
"time"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/registry"
"github.com/ghodss/yaml"
"gotest.tools/assert"

Expand Down Expand Up @@ -464,7 +464,7 @@ task_container_defaults:
SegmentWebUIKey: webuiSecret,
},
TaskContainerDefaults: model.TaskContainerDefaultsConfig{
RegistryAuth: &types.AuthConfig{
RegistryAuth: &registry.AuthConfig{
Username: "yo-yo-ma",
Password: registryAuthSecret,
},
Expand Down
5 changes: 3 additions & 2 deletions master/pkg/cproto/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/registry"

"github.com/determined-ai/determined/master/pkg/archive"
"github.com/determined-ai/determined/master/pkg/device"
Expand All @@ -19,7 +20,7 @@ type Spec struct {
// PullSpec contains configs for an ImagePull call.
type PullSpec struct {
ForcePull bool
Registry *types.AuthConfig
Registry *registry.AuthConfig
}

// RunSpec contains configs for ContainerCreate, CopyToContainer, and ContainerStart calls.
Expand All @@ -30,7 +31,7 @@ type RunSpec struct {

Archives []RunArchive
DeviceType device.Type
Registry *types.AuthConfig
Registry *registry.AuthConfig
}

// ChecksConfig describes the configuration for multiple readiness checks.
Expand Down
10 changes: 5 additions & 5 deletions master/pkg/model/environment_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/determined-ai/determined/master/pkg/check"
"github.com/determined-ai/determined/proto/pkg/apiv1"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/registry"
"github.com/pkg/errors"

"github.com/determined-ai/determined/master/pkg/device"
Expand All @@ -30,10 +30,10 @@ type Environment struct {
EnvironmentVariables RuntimeItems `json:"environment_variables,omitempty"`
ProxyPorts ProxyPortsConfig `json:"proxy_ports"`

Ports map[string]int `json:"ports"`
RegistryAuth *types.AuthConfig `json:"registry_auth,omitempty"`
ForcePullImage bool `json:"force_pull_image"`
PodSpec *k8sV1.Pod `json:"pod_spec"`
Ports map[string]int `json:"ports"`
RegistryAuth *registry.AuthConfig `json:"registry_auth,omitempty"`
ForcePullImage bool `json:"force_pull_image"`
PodSpec *k8sV1.Pod `json:"pod_spec"`

AddCapabilities []string `json:"add_capabilities"`
DropCapabilities []string `json:"drop_capabilities"`
Expand Down
10 changes: 5 additions & 5 deletions master/pkg/model/experiment_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"encoding/json"
"testing"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/registry"

"gotest.tools/assert"
)
Expand Down Expand Up @@ -79,15 +79,15 @@ func TestOverrideMasterConfigPullPolicy(t *testing.T) {

func TestMasterConfigRegistryAuth(t *testing.T) {
masterDefault := &TaskContainerDefaultsConfig{
RegistryAuth: &types.AuthConfig{
RegistryAuth: &registry.AuthConfig{
Username: "best-user",
Password: "secret-password",
},
}
actual := DefaultEnvConfig(masterDefault)

expected := DefaultEnvConfig(nil)
expected.RegistryAuth = &types.AuthConfig{
expected.RegistryAuth = &registry.AuthConfig{
Username: "best-user",
Password: "secret-password",
}
Expand All @@ -97,7 +97,7 @@ func TestMasterConfigRegistryAuth(t *testing.T) {

func TestOverrideMasterConfigRegistryAuth(t *testing.T) {
masterDefault := &TaskContainerDefaultsConfig{
RegistryAuth: &types.AuthConfig{
RegistryAuth: &registry.AuthConfig{
Username: "best-user",
},
}
Expand All @@ -107,7 +107,7 @@ func TestOverrideMasterConfigRegistryAuth(t *testing.T) {
}`), &actual))

expected := DefaultEnvConfig(nil)
expected.RegistryAuth = &types.AuthConfig{
expected.RegistryAuth = &registry.AuthConfig{
Username: "worst-user",
}

Expand Down
Loading

0 comments on commit 841d2ab

Please sign in to comment.