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

Jump to bottom

rkt: Fix docker auth config save directory to avoid race. #26177

Merged
merged 1 commit into from
May 31, 2016
Merged

rkt: Fix docker auth config save directory to avoid race. #26177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/kubelet/rkt/fake_rkt_interface_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func newFakeRktCli() *fakeRktCli {
}
}

func (f *fakeRktCli) RunCommand(args ...string) (result []string, err error) {
func (f *fakeRktCli) RunCommand(config *Config, args ...string) (result []string, err error) {
f.Lock()
defer f.Unlock()
cmd := append([]string{"rkt"}, args...)
Expand Down
30 changes: 15 additions & 15 deletions pkg/kubelet/rkt/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"io/ioutil"
"os"
"path"
"path/filepath"
"sort"
"strings"

Expand Down Expand Up @@ -62,13 +63,20 @@ func (r *Runtime) PullImage(image kubecontainer.ImageSpec, pullSecrets []api.Sec
glog.V(1).Infof("Pulling image %s without credentials", img)
}

// Let's update a json.
// TODO(yifan): Find a way to feed this to rkt.
if err := r.writeDockerAuthConfig(img, creds); err != nil {
userConfigDir, err := ioutil.TempDir("", "rktnetes-user-config-dir-")
if err != nil {
return fmt.Errorf("rkt: Cannot create a temporary user-config directory: %v", err)
}
defer os.RemoveAll(userConfigDir)

config := *r.config
config.UserConfigDir = userConfigDir
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this override actually needed for anything other than the fetch immediately following? It seems like this could be simplified to just pass in "fetch", "--user-config=$dir" below and not make further changes.

Is the goal for this to also eventually do more? Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@euank passing the "fetch", "--user-config=$dir" doesn't precludes the buildCommand or RunCommand from generating --user-config for another time.


if err := r.writeDockerAuthConfig(img, creds, userConfigDir); err != nil {
return err
}

if _, err := r.cli.RunCommand("fetch", dockerPrefix+img); err != nil {
if _, err := r.cli.RunCommand(&config, "fetch", dockerPrefix+img); err != nil {
glog.Errorf("Failed to fetch: %v", err)
return err
}
Expand Down Expand Up @@ -104,7 +112,7 @@ func (r *Runtime) RemoveImage(image kubecontainer.ImageSpec) error {
if err != nil {
return err
}
if _, err := r.cli.RunCommand("image", "rm", imageID); err != nil {
if _, err := r.cli.RunCommand(nil, "image", "rm", imageID); err != nil {
return err
}
return nil
Expand Down Expand Up @@ -186,7 +194,7 @@ func (r *Runtime) getImageManifest(image string) (*appcschema.ImageManifest, err

// TODO(yifan): This is very racy, unefficient, and unsafe, we need to provide
// different namespaces. See: https://github.com/coreos/rkt/issues/836.
func (r *Runtime) writeDockerAuthConfig(image string, credsSlice []credentialprovider.LazyAuthConfiguration) error {
func (r *Runtime) writeDockerAuthConfig(image string, credsSlice []credentialprovider.LazyAuthConfiguration, userConfigDir string) error {
if len(credsSlice) == 0 {
return nil
}
Expand All @@ -204,15 +212,7 @@ func (r *Runtime) writeDockerAuthConfig(image string, credsSlice []credentialpro
registry = strings.Split(image, "/")[0]
}

configDir := r.config.UserConfigDir
if configDir == "" {
configDir = r.config.LocalConfigDir
}
if configDir == "" {
return fmt.Errorf("No user or local config dir is specified")
}

authDir := path.Join(configDir, "auth.d")
authDir := filepath.Join(userConfigDir, "auth.d")
if _, err := os.Stat(authDir); os.IsNotExist(err) {
if err := os.MkdirAll(authDir, 0600); err != nil {
glog.Errorf("rkt: Cannot create auth dir: %v", err)
Expand Down
37 changes: 22 additions & 15 deletions pkg/kubelet/rkt/rkt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,9 @@ type podGetter interface {

// cliInterface wrapps the command line calls for testing purpose.
type cliInterface interface {
// args are the arguments given to the 'rkt' command,
// e.g. args can be 'rm ${UUID}'.
RunCommand(args ...string) (result []string, err error)
// RunCommand creates rkt commands and runs it with the given config.
// If the config is nil, it will use the one inferred from rkt API service.
RunCommand(config *Config, args ...string) (result []string, err error)
}

// New creates the rkt container runtime which implements the container runtime interface.
Expand Down Expand Up @@ -263,9 +263,11 @@ func New(
return rkt, nil
}

func (r *Runtime) buildCommand(args ...string) *exec.Cmd {
allArgs := append(r.config.buildGlobalOptions(), args...)
return exec.Command(r.config.Path, allArgs...)
func buildCommand(config *Config, args ...string) *exec.Cmd {
cmd := exec.Command(config.Path)
cmd.Args = append(cmd.Args, config.buildGlobalOptions()...)
cmd.Args = append(cmd.Args, args...)
return cmd
}

// convertToACName converts a string into ACName.
Expand All @@ -278,13 +280,18 @@ func convertToACName(name string) appctypes.ACName {

// RunCommand invokes rkt binary with arguments and returns the result
// from stdout in a list of strings. Each string in the list is a line.
func (r *Runtime) RunCommand(args ...string) ([]string, error) {
glog.V(4).Info("rkt: Run command:", args)
// If config is non-nil, it will use the given config instead of the config
// inferred from rkt API service.
func (r *Runtime) RunCommand(config *Config, args ...string) ([]string, error) {
if config == nil {
config = r.config
}
glog.V(4).Infof("rkt: Run command: %q with config: %+v", args, config)

var stdout, stderr bytes.Buffer
cmd := r.buildCommand(args...)
cmd.Stdout = &stdout
cmd.Stderr = &stderr

cmd := buildCommand(config, args...)
cmd.Stdout, cmd.Stderr = &stdout, &stderr
if err := cmd.Run(); err != nil {
return nil, fmt.Errorf("failed to run %v: %v\nstdout: %v\nstderr: %v", args, err, stdout.String(), stderr.String())
}
Expand Down Expand Up @@ -895,7 +902,7 @@ func serviceFilePath(serviceName string) string {

// generateRunCommand crafts a 'rkt run-prepared' command with necessary parameters.
func (r *Runtime) generateRunCommand(pod *api.Pod, uuid, netnsName string) (string, error) {
runPrepared := r.buildCommand("run-prepared").Args
runPrepared := buildCommand(r.config, "run-prepared").Args

// Network namespace set up in kubelet; rkt networking not used
runPrepared = append(runPrepared, "--net=host")
Expand Down Expand Up @@ -1019,7 +1026,7 @@ func (r *Runtime) preparePod(pod *api.Pod, podIP string, pullSecrets []api.Secre
}

prepareCmd := r.preparePodArgs(manifest, manifestFile.Name())
output, err := r.RunCommand(prepareCmd...)
output, err := r.cli.RunCommand(nil, prepareCmd...)
if err != nil {
return "", nil, err
}
Expand Down Expand Up @@ -1826,7 +1833,7 @@ func (r *Runtime) removePod(uuid string) error {
// Network may not be around anymore so errors are ignored
r.cleanupPodNetworkFromServiceFile(serviceFile)

if _, err := r.cli.RunCommand("rm", uuid); err != nil {
if _, err := r.cli.RunCommand(nil, "rm", uuid); err != nil {
errlist = append(errlist, fmt.Errorf("rkt: Failed to remove pod %q: %v", uuid, err))
}

Expand Down Expand Up @@ -1866,7 +1873,7 @@ func (r *Runtime) ExecInContainer(containerID kubecontainer.ContainerID, cmd []s
}
args := []string{"enter", fmt.Sprintf("--app=%s", id.appName), id.uuid}
args = append(args, cmd...)
command := r.buildCommand(args...)
command := buildCommand(r.config, args...)

if tty {
p, err := kubecontainer.StartPty(command)
Expand Down