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.

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

More small refactors for v1beta3 -> internal #2912

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 0 additions & 19 deletions pkg/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,25 +435,6 @@ type RestartPolicy struct {
Never *RestartPolicyNever `json:"never,omitempty"`
}

// PodState is the state of a pod, used as either input (desired state) or output (current state).
type PodState struct {
Manifest ContainerManifest `json:"manifest,omitempty"`
Status PodPhase `json:"status,omitempty"`
// A human readable message indicating details about why the pod is in this state.
Message string `json:"message,omitempty"`
Host string `json:"host,omitempty"`
HostIP string `json:"hostIP,omitempty"`
PodIP string `json:"podIP,omitempty"`

// The key of this map is the *name* of the container within the manifest; it has one
// entry per container in the manifest. The value of this map is currently the output
// of `docker inspect`. This output format is *not* final and should not be relied
// upon.
// TODO: Make real decisions about what our info should look like. Re-enable fuzz test
// when we have done this.
Info PodInfo `json:"info,omitempty"`
}

// PodList is a list of Pods.
type PodList struct {
TypeMeta `json:",inline"`
Expand Down
12 changes: 12 additions & 0 deletions pkg/api/v1beta1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,18 @@ func init() {
out.PodIP = in.PodIP
return nil
},
func(in *newer.PodSpec, out *PodState, s conversion.Scope) error {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be PodStatus rather than PodSpec?

if err := s.Convert(&in, &out.Manifest, 0); err != nil {
return err
}
return nil
},
func(in *PodState, out *newer.PodSpec, s conversion.Scope) error {
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Legacy- that's v1beta1 and 2

On Dec 12, 2014, at 8:19 PM, bgrant0607 notifications@github.com wrote:

In pkg/api/v1beta1/conversion.go:

@@ -194,6 +194,18 @@ func init() {
out.PodIP = in.PodIP
return nil
},

  •   func(in *newer.PodSpec, out *PodState, s conversion.Scope) error {
    
  •       if err := s.Convert(&in, &out.Manifest, 0); err != nil {
    
  •           return err
    
  •       }
    
  •       return nil
    
  •   },
    
  •   func(in *PodState, out *newer.PodSpec, s conversion.Scope) error {
    
    Ditto.


Reply to this email directly or view it on GitHub.

if err := s.Convert(&in.Manifest, &out, 0); err != nil {
return err
}
return nil
},

// Convert all to the new PodPhase constants
func(in *newer.PodPhase, out *PodStatus, s conversion.Scope) error {
Expand Down
9 changes: 6 additions & 3 deletions pkg/api/v1beta3/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,6 @@ type ExecAction struct {
// LivenessProbe describes how to probe a container for liveness.
// TODO: pass structured data to the actions, and document that data here.
type LivenessProbe struct {
// Type of liveness probe. Current legal values "HTTP", "TCP", "Exec"
Type string `json:"type,omitempty"`
// HTTPGetProbe parameters, required if Type == 'HTTP'
HTTPGet *HTTPGetAction `json:"httpGet,omitempty"`
// TCPSocketProbe parameter, required if Type == 'TCP'
Expand Down Expand Up @@ -420,8 +418,13 @@ type ContainerStatus struct {
// garbage collection. This value will get capped at 5 by GC.
RestartCount int `json:"restartCount"`
// TODO(dchen1107): Introduce our own NetworkSettings struct here?
// TODO(dchen1107): Which image the container is running with?
ContainerID string `json:"containerID,omitempty" description:"container's ID in the format 'docker://<container_id>'"`
// The IP of the Pod
// PodIP is deprecated and will be removed from v1beta3 once it becomes possible for the Kubelet to report PodStatus.
PodIP string `json:"podIP,omitempty"`
// TODO(dchen1107): Which image the container is running with?
// The image the container is running
Image string `json:"image"`
}

// PodInfo contains one entry for every container with available info.
Expand Down
5 changes: 0 additions & 5 deletions pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,6 @@ func validateRestartPolicy(restartPolicy *api.RestartPolicy) errs.ValidationErro
return allErrors
}

func ValidatePodState(podState *api.PodState) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList(ValidateManifest(&podState.Manifest)).Prefix("manifest")
return allErrs
}

// ValidatePod tests if required fields in the pod are set.
func ValidatePod(pod *api.Pod) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
Expand Down
2 changes: 1 addition & 1 deletion pkg/health/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func NewExecHealthChecker(runner CommandRunner) HealthChecker {
return &ExecHealthChecker{runner}
}

func (e *ExecHealthChecker) HealthCheck(podFullName, podUUID string, currentState api.PodState, container api.Container) (Status, error) {
func (e *ExecHealthChecker) HealthCheck(podFullName, podUUID string, status api.PodStatus, container api.Container) (Status, error) {
if container.LivenessProbe.Exec == nil {
return Unknown, fmt.Errorf("missing exec parameters")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/health/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestExec(t *testing.T) {
for _, test := range tests {
fake.out = test.output
fake.err = test.err
status, err := checker.HealthCheck("test", "", api.PodState{}, api.Container{LivenessProbe: test.probe})
status, err := checker.HealthCheck("test", "", api.PodStatus{}, api.Container{LivenessProbe: test.probe})
if status != test.expectedStatus {
t.Errorf("expected %v, got %v", test.expectedStatus, status)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/health/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const (

// HealthChecker defines an abstract interface for checking container health.
type HealthChecker interface {
HealthCheck(podFullName, podUUID string, currentState api.PodState, container api.Container) (Status, error)
HealthCheck(podFullName, podUUID string, status api.PodStatus, container api.Container) (Status, error)
CanCheck(probe *api.LivenessProbe) bool
}

Expand Down Expand Up @@ -78,13 +78,13 @@ func (m *muxHealthChecker) findCheckerFor(probe *api.LivenessProbe) HealthChecke

// HealthCheck delegates the health-checking of the container to one of the bundled implementations.
// If there is no health checker that can check container it returns Unknown, nil.
func (m *muxHealthChecker) HealthCheck(podFullName, podUUID string, currentState api.PodState, container api.Container) (Status, error) {
func (m *muxHealthChecker) HealthCheck(podFullName, podUUID string, status api.PodStatus, container api.Container) (Status, error) {
checker := m.findCheckerFor(container.LivenessProbe)
if checker == nil {
glog.Warningf("Failed to find health checker for %s %+v", container.Name, container.LivenessProbe)
return Unknown, nil
}
return checker.HealthCheck(podFullName, podUUID, currentState, container)
return checker.HealthCheck(podFullName, podUUID, status, container)
}

func (m *muxHealthChecker) CanCheck(probe *api.LivenessProbe) bool {
Expand Down
4 changes: 2 additions & 2 deletions pkg/health/health_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestHealthChecker(t *testing.T) {
},
}
hc := NewHealthChecker()
health, err := hc.HealthCheck("test", "", api.PodState{}, container)
health, err := hc.HealthCheck("test", "", api.PodStatus{}, container)
if err != nil && tt.health != Unknown {
t.Errorf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -131,7 +131,7 @@ func TestMuxHealthChecker(t *testing.T) {
}
container.LivenessProbe.HTTPGet.Port = util.NewIntOrStringFromString(port)
container.LivenessProbe.HTTPGet.Host = host
health, err := mc.HealthCheck("test", "", api.PodState{}, container)
health, err := mc.HealthCheck("test", "", api.PodStatus{}, container)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/health/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewHTTPHealthChecker(client *http.Client) HealthChecker {
}

// getURLParts parses the components of the target URL. For testability.
func getURLParts(currentState api.PodState, container api.Container) (string, int, string, error) {
func getURLParts(status api.PodStatus, container api.Container) (string, int, string, error) {
params := container.LivenessProbe.HTTPGet
if params == nil {
return "", -1, "", fmt.Errorf("no HTTP parameters specified: %v", container)
Expand All @@ -70,7 +70,7 @@ func getURLParts(currentState api.PodState, container api.Container) (string, in
if len(params.Host) > 0 {
host = params.Host
} else {
host = currentState.PodIP
host = status.PodIP
}

return host, port, params.Path, nil
Expand Down Expand Up @@ -105,8 +105,8 @@ func DoHTTPCheck(url string, client HTTPGetInterface) (Status, error) {
}

// HealthCheck checks if the container is healthy by trying sending HTTP Get requests to the container.
func (h *HTTPHealthChecker) HealthCheck(podFullName, podUUID string, currentState api.PodState, container api.Container) (Status, error) {
host, port, path, err := getURLParts(currentState, container)
func (h *HTTPHealthChecker) HealthCheck(podFullName, podUUID string, status api.PodStatus, container api.Container) (Status, error) {
host, port, path, err := getURLParts(status, container)
if err != nil {
return Unknown, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/health/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestGetURLParts(t *testing.T) {
}

for _, test := range testCases {
state := api.PodState{PodIP: "127.0.0.1"}
state := api.PodStatus{PodIP: "127.0.0.1"}
container := api.Container{
Ports: []api.Port{{Name: "found", HostPort: 93}},
LivenessProbe: &api.LivenessProbe{
Expand Down Expand Up @@ -123,7 +123,7 @@ func TestHTTPHealthChecker(t *testing.T) {
params.Port = util.NewIntOrStringFromString(port)
params.Host = host
}
health, err := hc.HealthCheck("test", "", api.PodState{PodIP: host}, container)
health, err := hc.HealthCheck("test", "", api.PodStatus{PodIP: host}, container)
if test.health == Unknown && err == nil {
t.Errorf("Expected error")
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/health/tcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
type TCPHealthChecker struct{}

// getTCPAddrParts parses the components of a TCP connection address. For testability.
func getTCPAddrParts(currentState api.PodState, container api.Container) (string, int, error) {
func getTCPAddrParts(status api.PodStatus, container api.Container) (string, int, error) {
params := container.LivenessProbe.TCPSocket
if params == nil {
return "", -1, fmt.Errorf("error, no TCP parameters specified: %v", container)
Expand All @@ -51,11 +51,11 @@ func getTCPAddrParts(currentState api.PodState, container api.Container) (string
if port == -1 {
return "", -1, fmt.Errorf("unknown port: %v", params.Port)
}
if len(currentState.PodIP) == 0 {
if len(status.PodIP) == 0 {
return "", -1, fmt.Errorf("no host specified.")
}

return currentState.PodIP, port, nil
return status.PodIP, port, nil
}

// DoTCPCheck checks that a TCP socket to the address can be opened.
Expand All @@ -74,8 +74,8 @@ func DoTCPCheck(addr string) (Status, error) {
return Healthy, nil
}

func (t *TCPHealthChecker) HealthCheck(podFullName, podUUID string, currentState api.PodState, container api.Container) (Status, error) {
host, port, err := getTCPAddrParts(currentState, container)
func (t *TCPHealthChecker) HealthCheck(podFullName, podUUID string, status api.PodStatus, container api.Container) (Status, error) {
host, port, err := getTCPAddrParts(status, container)
if err != nil {
return Unknown, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/health/tcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestGetTCPAddrParts(t *testing.T) {
}

for _, test := range testCases {
state := api.PodState{PodIP: "1.2.3.4"}
state := api.PodStatus{PodIP: "1.2.3.4"}
container := api.Container{
Ports: []api.Port{{Name: "found", HostPort: 93}},
LivenessProbe: &api.LivenessProbe{
Expand Down Expand Up @@ -101,7 +101,7 @@ func TestTcpHealthChecker(t *testing.T) {
if params != nil && test.expectedStatus == Healthy {
params.Port = util.NewIntOrStringFromString(port)
}
status, err := checker.HealthCheck("test", "", api.PodState{PodIP: host}, container)
status, err := checker.HealthCheck("test", "", api.PodStatus{PodIP: host}, container)
if status != test.expectedStatus {
t.Errorf("expected: %v, got: %v", test.expectedStatus, status)
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/kubelet/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,14 +703,14 @@ func (kl *Kubelet) syncPod(pod *api.BoundPod, dockerContainers dockertools.Docke
return err
}

podState := api.PodState{}
podStatus := api.PodStatus{}
info, err := kl.GetPodInfo(podFullName, uuid)
if err != nil {
glog.Errorf("Unable to get pod with name %s and uuid %s info, health checks may be invalid", podFullName, uuid)
}
netInfo, found := info[networkContainerName]
if found {
podState.PodIP = netInfo.PodIP
podStatus.PodIP = netInfo.PodIP
}

for _, container := range pod.Spec.Containers {
Expand All @@ -722,7 +722,7 @@ func (kl *Kubelet) syncPod(pod *api.BoundPod, dockerContainers dockertools.Docke
// look for changes in the container.
if hash == 0 || hash == expectedHash {
// TODO: This should probably be separated out into a separate goroutine.
healthy, err := kl.healthy(podFullName, uuid, podState, container, dockerContainer)
healthy, err := kl.healthy(podFullName, uuid, podStatus, container, dockerContainer)
if err != nil {
glog.V(1).Infof("health check errored: %v", err)
containersToKeep[containerID] = empty{}
Expand Down Expand Up @@ -1042,7 +1042,7 @@ func (kl *Kubelet) GetPodInfo(podFullName, uuid string) (api.PodInfo, error) {
return dockertools.GetDockerPodInfo(kl.dockerClient, manifest, podFullName, uuid)
}

func (kl *Kubelet) healthy(podFullName, podUUID string, currentState api.PodState, container api.Container, dockerContainer *docker.APIContainers) (health.Status, error) {
func (kl *Kubelet) healthy(podFullName, podUUID string, status api.PodStatus, container api.Container, dockerContainer *docker.APIContainers) (health.Status, error) {
// Give the container 60 seconds to start up.
if container.LivenessProbe == nil {
return health.Healthy, nil
Expand All @@ -1053,7 +1053,7 @@ func (kl *Kubelet) healthy(podFullName, podUUID string, currentState api.PodStat
if kl.healthChecker == nil {
return health.Healthy, nil
}
return kl.healthChecker.HealthCheck(podFullName, podUUID, currentState, container)
return kl.healthChecker.HealthCheck(podFullName, podUUID, status, container)
}

// Returns logs of current machine.
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/kubelet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ func TestSyncPodDeletesDuplicate(t *testing.T) {

type FalseHealthChecker struct{}

func (f *FalseHealthChecker) HealthCheck(podFullName, podUUID string, state api.PodState, container api.Container) (health.Status, error) {
func (f *FalseHealthChecker) HealthCheck(podFullName, podUUID string, status api.PodStatus, container api.Container) (health.Status, error) {
return health.Unhealthy, nil
}

Expand Down