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

Add the ability to turn off image pulling. #1458

Merged
merged 1 commit into from
Oct 1, 2014
Merged

Add the ability to turn off image pulling. #1458

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
36 changes: 36 additions & 0 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package api

import (
"strings"

"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/fsouza/go-dockerclient"
)
Expand Down Expand Up @@ -173,6 +175,38 @@ type LivenessProbe struct {
InitialDelaySeconds int64 `yaml:"initialDelaySeconds,omitempty" json:"initialDelaySeconds,omitempty"`
}

// PullPolicy describes a policy for if/when to pull a container image
type PullPolicy string

const (
// Always attempt to pull the latest image. Container will fail If the pull fails.
PullAlways PullPolicy = "PullAlways"
// Never pull an image, only use a local image. Container will fail if the image isn't present
PullNever PullPolicy = "PullNever"
// Pull if the image isn't present on disk. Container will fail if the image isn't present and the pull fails.
PullIfNotPresent PullPolicy = "PullIfNotPresent"
)

func IsPullAlways(p PullPolicy) bool {
// Default to pull always
if len(p) == 0 {
return true
}
return pullPoliciesEqual(p, PullAlways)
Copy link
Member

Choose a reason for hiding this comment

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

along the way we lost the ToUpper() or ToLower() to make these case-insensitive.

As a different question: This pattern (IsFoo() functions which encode the if-not-specified behavior) vs defaulting values in validation? This pattern has a distinct advantage of working in tests where validation has not run. It has the distinct disadvantage that not all fields are so self-contained - some set their default based on another field's value.

I'd like to be consistent as much as possible. I'll file an issue. No urgency.

Copy link
Member

Choose a reason for hiding this comment

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

Other disadvantages:

  • Clients can't introspect default behavior
  • Creates a subtle API version dependency: Changing default behavior is a breaking API change. If we were to ever change such defaults, we'd need to factor out the code into a version-specific location. Furthermore, if we were to allow objects to survive API version changes (which we probably will eventually), we'd want to record the original default in the object.

}

func IsPullNever(p PullPolicy) bool {
return pullPoliciesEqual(p, PullNever)
}

func IsPullIfNotPresent(p PullPolicy) bool {
return pullPoliciesEqual(p, PullIfNotPresent)
}

func pullPoliciesEqual(p1, p2 PullPolicy) bool {
return strings.ToLower(string(p1)) == strings.ToLower(string(p2))
}

// Container represents a single container that is expected to be run on the host.
type Container struct {
// Required: This must be a DNS_LABEL. Each container in a pod must
Expand All @@ -195,6 +229,8 @@ type Container struct {
Lifecycle *Lifecycle `yaml:"lifecycle,omitempty" json:"lifecycle,omitempty"`
// Optional: Default to false.
Privileged bool `json:"privileged,omitempty" yaml:"privileged,omitempty"`
// Optional: Policy for pulling images for this container
ImagePullPolicy PullPolicy `json:"imagePullPolicy" yaml:"imagePullPolicy"`
}

// Handler defines a specific action that should be taken
Expand Down
14 changes: 14 additions & 0 deletions pkg/api/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,18 @@ type LivenessProbe struct {
InitialDelaySeconds int64 `yaml:"initialDelaySeconds,omitempty" json:"initialDelaySeconds,omitempty"`
}

// PullPolicy describes a policy for if/when to pull a container image
type PullPolicy string

const (
// Always attempt to pull the latest image. Container will fail If the pull fails.
PullAlways PullPolicy = "PullAlways"
// Never pull an image, only use a local image. Container will fail if the image isn't present
PullNever PullPolicy = "PullNever"
// Pull if the image isn't present on disk. Container will fail if the image isn't present and the pull fails.
PullIfNotPresent PullPolicy = "PullIfNotPresent"
)

// Container represents a single container that is expected to be run on the host.
type Container struct {
// Required: This must be a DNS_LABEL. Each container in a pod must
Expand All @@ -205,6 +217,8 @@ type Container struct {
Lifecycle *Lifecycle `yaml:"lifecycle,omitempty" json:"lifecycle,omitempty"`
// Optional: Default to false.
Privileged bool `json:"privileged,omitempty" yaml:"privileged,omitempty"`
// Optional: Policy for pulling images for this container
ImagePullPolicy PullPolicy `json:"imagePullPolicy" yaml:"imagePullPolicy"`
}

// Handler defines a specific action that should be taken
Expand Down
14 changes: 14 additions & 0 deletions pkg/api/v1beta2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,18 @@ type LivenessProbe struct {
InitialDelaySeconds int64 `yaml:"initialDelaySeconds,omitempty" json:"initialDelaySeconds,omitempty"`
}

// PullPolicy describes a policy for if/when to pull a container image
type PullPolicy string

const (
// Always attempt to pull the latest image. Container will fail If the pull fails.
PullAlways PullPolicy = "PullAlways"
// Never pull an image, only use a local image. Container will fail if the image isn't present
PullNever PullPolicy = "PullNever"
// Pull if the image isn't present on disk. Container will fail if the image isn't present and the pull fails.
PullIfNotPresent PullPolicy = "PullIfNotPresent"
)

// Container represents a single container that is expected to be run on the host.
type Container struct {
// Required: This must be a DNS_LABEL. Each container in a pod must
Expand All @@ -204,6 +216,8 @@ type Container struct {
Lifecycle *Lifecycle `yaml:"lifecycle,omitempty" json:"lifecycle,omitempty"`
// Optional: Default to false.
Privileged bool `json:"privileged,omitempty" yaml:"privileged,omitempty"`
// Optional: Policy for pulling images for this container
ImagePullPolicy PullPolicy `json:"imagePullPolicy" yaml:"imagePullPolicy"`
}

// Handler defines a specific action that should be taken
Expand Down
14 changes: 14 additions & 0 deletions pkg/api/v1beta3/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,18 @@ type LivenessProbe struct {
InitialDelaySeconds int64 `yaml:"initialDelaySeconds,omitempty" json:"initialDelaySeconds,omitempty"`
}

// PullPolicy describes a policy for if/when to pull a container image
type PullPolicy string

const (
// Always attempt to pull the latest image. Container will fail If the pull fails.
PullAlways PullPolicy = "PullAlways"
// Never pull an image, only use a local image. Container will fail if the image isn't present
PullNever PullPolicy = "PullNever"
// Pull if the image isn't present on disk. Container will fail if the image isn't present and the pull fails.
PullIfNotPresent PullPolicy = "PullIfNotPresent"
)

// Container represents a single container that is expected to be run on the host.
type Container struct {
// Required: This must be a DNS_LABEL. Each container in a pod must
Expand All @@ -195,6 +207,8 @@ type Container struct {
Lifecycle *Lifecycle `yaml:"lifecycle,omitempty" json:"lifecycle,omitempty"`
// Optional: Default to false.
Privileged bool `json:"privileged,omitempty" yaml:"privileged,omitempty"`
// Optional: Policy for pulling images for this container
ImagePullPolicy PullPolicy `json:"imagePullPolicy" yaml:"imagePullPolicy"`
}

// Handler defines a specific action that should be taken
Expand Down
20 changes: 20 additions & 0 deletions pkg/kubelet/dockertools/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type DockerInterface interface {
CreateContainer(docker.CreateContainerOptions) (*docker.Container, error)
StartContainer(id string, hostConfig *docker.HostConfig) error
StopContainer(id string, timeout uint) error
InspectImage(image string) (*docker.Image, error)
PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error
Logs(opts docker.LogsOptions) error
}
Expand All @@ -57,6 +58,7 @@ type DockerID string
// DockerPuller is an abstract interface for testability. It abstracts image pull operations.
type DockerPuller interface {
Pull(image string) error
IsImagePresent(image string) (bool, error)
}

// dockerPuller is the default implementation of DockerPuller.
Expand Down Expand Up @@ -148,6 +150,24 @@ func (p throttledDockerPuller) Pull(image string) error {
return fmt.Errorf("pull QPS exceeded.")
}

func (p dockerPuller) IsImagePresent(name string) (bool, error) {
image, _ := parseImageName(name)
_, err := p.client.InspectImage(image)
if err == nil {
return true, nil
}
// This is super brittle, but its the best we got.
// TODO: Land code in the docker client to use docker.Error here instead.
if err.Error() == "no such image" {
return false, nil
}
return false, err
}

func (p throttledDockerPuller) IsImagePresent(name string) (bool, error) {
return p.puller.IsImagePresent(name)
}

// DockerContainers is a map of containers
type DockerContainers map[DockerID]*docker.APIContainers

Expand Down
8 changes: 8 additions & 0 deletions pkg/kubelet/dockertools/fake_docker_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ func (f *FakeDockerClient) PullImage(opts docker.PullImageOptions, auth docker.A
return f.Err
}

func (f *FakeDockerClient) InspectImage(name string) (*docker.Image, error) {
return nil, f.Err
}

// FakeDockerPuller is a stub implementation of DockerPuller.
type FakeDockerPuller struct {
sync.Mutex
Expand All @@ -153,3 +157,7 @@ func (f *FakeDockerPuller) Pull(image string) (err error) {
}
return err
}

func (f *FakeDockerPuller) IsImagePresent(name string) (bool, error) {
return true, nil
}
15 changes: 12 additions & 3 deletions pkg/kubelet/kubelet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,18 @@ func (kl *Kubelet) syncPod(pod *Pod, dockerContainers dockertools.DockerContaine
}

glog.V(3).Infof("Container with name %s--%s--%s doesn't exist, creating %#v", podFullName, uuid, container.Name, container)
if err := kl.dockerPuller.Pull(container.Image); err != nil {
glog.Errorf("Failed to pull image %s: %v skipping pod %s container %s.", container.Image, err, podFullName, container.Name)
continue
if !api.IsPullNever(container.ImagePullPolicy) {
present, err := kl.dockerPuller.IsImagePresent(container.Image)
if err != nil {
glog.Errorf("Failed to inspect image: %s: %#v skipping pod %s container %s", container.Image, err, podFullName, container.Name)
continue
}
if api.IsPullAlways(container.ImagePullPolicy) || !present {
if err := kl.dockerPuller.Pull(container.Image); err != nil {
glog.Errorf("Failed to pull image %s: %v skipping pod %s container %s.", container.Image, err, podFullName, container.Name)
continue
}
}
}
// TODO(dawnchen): Check RestartPolicy.DelaySeconds before restart a container
containerID, err := kl.runContainer(pod, &container, podVolumes, "container:"+string(netID))
Expand Down