Skip to content

Commit

Permalink
change *proto.ActionableErr to proto.ActionableErr
Browse files Browse the repository at this point in the history
  • Loading branch information
tejal29 committed Jun 25, 2020
1 parent 9578754 commit 270899b
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 87 deletions.
26 changes: 16 additions & 10 deletions pkg/diag/validator/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,15 @@ func processPodEvents(e corev1.EventInterface, pod v1.Pod, ps *podStatus) {
}
switch recentEvent.Reason {
case failedScheduling:
ps.ae.ErrCode = proto.StatusCode_STATUSCHECK_FAILED_SCHEDULING
ps.ae.Message = recentEvent.Message
ps.updateAE(proto.StatusCode_STATUSCHECK_FAILED_SCHEDULING, recentEvent.Message)
case unhealthy:
ps.ae.ErrCode = proto.StatusCode_STATUSCHECK_UNHEALTHY
ps.ae.Message = recentEvent.Message
ps.updateAE(proto.StatusCode_STATUSCHECK_UNHEALTHY, recentEvent.Message)
default:
// TODO: Add unique error codes for reasons
ps.ae.ErrCode = proto.StatusCode_STATUSCHECK_UNKNOWN_EVENT
ps.ae.Message = fmt.Sprintf("%s: %s", recentEvent.Reason, recentEvent.Message)
ps.updateAE(
proto.StatusCode_STATUSCHECK_UNKNOWN_EVENT,
fmt.Sprintf("%s: %s", recentEvent.Reason, recentEvent.Message),
)
}
}

Expand All @@ -226,22 +226,28 @@ type podStatus struct {
namespace string
phase string
logs []string
ae *proto.ActionableErr
ae proto.ActionableErr
}

func (p *podStatus) isStable() bool {
return p.phase == success || (p.phase == running && p.ae.Message == "")
}

func (p *podStatus) withErrAndLogs(errCode proto.StatusCode, l []string, err error) *podStatus {
var msg string
if err != nil {
p.ae.Message = err.Error()
msg = err.Error()
}
p.ae.ErrCode = errCode
p.updateAE(errCode, msg)
p.logs = l
return p
}

func (p *podStatus) updateAE(errCode proto.StatusCode, msg string) {
p.ae.ErrCode = errCode
p.ae.Message = msg
}

func (p *podStatus) String() string {
switch {
case p.isStable():
Expand Down Expand Up @@ -279,7 +285,7 @@ func newPodStatus(n string, ns string, p string) *podStatus {
name: n,
namespace: ns,
phase: p,
ae: &proto.ActionableErr{
ae: proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_SUCCESS,
},
}
Expand Down
26 changes: 13 additions & 13 deletions pkg/diag/validator/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestRun(t *testing.T) {
},
}},
expected: []Resource{NewResource("test", "Pod", "foo", "Pending",
&proto.ActionableErr{
proto.ActionableErr{
Message: "container foo-container is waiting to start: foo-image can't be pulled",
ErrCode: proto.StatusCode_STATUSCHECK_IMAGE_PULL_ERR,
}, nil)},
Expand Down Expand Up @@ -115,7 +115,7 @@ func TestRun(t *testing.T) {
},
}},
expected: []Resource{NewResource("test", "Pod", "foo", "Pending",
&proto.ActionableErr{
proto.ActionableErr{
Message: "container foo-container is waiting to start: foo-image can't be pulled",
ErrCode: proto.StatusCode_STATUSCHECK_IMAGE_PULL_ERR,
}, nil)},
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestRun(t *testing.T) {
},
}},
expected: []Resource{NewResource("test", "Pod", "foo", "Pending",
&proto.ActionableErr{
proto.ActionableErr{
Message: "container foo-container is waiting to start: foo-image can't be pulled",
ErrCode: proto.StatusCode_STATUSCHECK_IMAGE_PULL_ERR,
}, nil)},
Expand All @@ -165,7 +165,7 @@ func TestRun(t *testing.T) {
},
}},
expected: []Resource{NewResource("test", "Pod", "foo", "Succeeded",
&proto.ActionableErr{
proto.ActionableErr{
Message: "",
ErrCode: proto.StatusCode_STATUSCHECK_SUCCESS,
}, nil)},
Expand All @@ -190,7 +190,7 @@ func TestRun(t *testing.T) {
},
}},
expected: []Resource{NewResource("test", "Pod", "foo", "Running",
&proto.ActionableErr{
proto.ActionableErr{
Message: "",
ErrCode: proto.StatusCode_STATUSCHECK_SUCCESS,
}, nil)},
Expand All @@ -213,7 +213,7 @@ func TestRun(t *testing.T) {
},
}},
expected: []Resource{NewResource("test", "Pod", "foo", "Pending",
&proto.ActionableErr{
proto.ActionableErr{
Message: "could not determine",
ErrCode: proto.StatusCode_STATUSCHECK_UNKNOWN,
}, nil)},
Expand All @@ -237,7 +237,7 @@ func TestRun(t *testing.T) {
},
}},
expected: []Resource{NewResource("test", "Pod", "foo", "Pending",
&proto.ActionableErr{
proto.ActionableErr{
Message: "Unschedulable: 0/2 nodes available: 1 node has disk pressure, 1 node is unreachable",
ErrCode: proto.StatusCode_STATUSCHECK_NODE_DISK_PRESSURE,
}, nil)},
Expand Down Expand Up @@ -265,7 +265,7 @@ func TestRun(t *testing.T) {
output: []byte("main.go:57 \ngo panic"),
},
expected: []Resource{NewResource("test", "Pod", "foo", "Running",
&proto.ActionableErr{
proto.ActionableErr{
Message: "container foo-container terminated with exit code 1",
ErrCode: proto.StatusCode_STATUSCHECK_CONTAINER_TERMINATED,
}, []string{
Expand Down Expand Up @@ -295,7 +295,7 @@ func TestRun(t *testing.T) {
err: fmt.Errorf("error"),
},
expected: []Resource{NewResource("test", "pod", "foo", "Running",
&proto.ActionableErr{
proto.ActionableErr{
Message: "container foo-container terminated with exit code 1",
ErrCode: proto.StatusCode_STATUSCHECK_CONTAINER_TERMINATED,
}, []string{
Expand Down Expand Up @@ -327,7 +327,7 @@ func TestRun(t *testing.T) {
},
},
expected: []Resource{NewResource("test", "Pod", "foo", "Pending",
&proto.ActionableErr{
proto.ActionableErr{
Message: "eventCode: dummy event",
ErrCode: proto.StatusCode_STATUSCHECK_UNKNOWN_EVENT,
}, nil)},
Expand Down Expand Up @@ -362,7 +362,7 @@ func TestRun(t *testing.T) {
},
},
expected: []Resource{NewResource("test", "Pod", "foo", "Pending",
&proto.ActionableErr{
proto.ActionableErr{
Message: "eventCode: dummy event",
ErrCode: proto.StatusCode_STATUSCHECK_UNKNOWN_EVENT,
}, nil)},
Expand Down Expand Up @@ -397,7 +397,7 @@ func TestRun(t *testing.T) {
},
},
expected: []Resource{NewResource("test", "Pod", "foo", "Pending",
&proto.ActionableErr{
proto.ActionableErr{
Message: "eventCode: dummy event",
ErrCode: proto.StatusCode_STATUSCHECK_UNKNOWN_EVENT,
}, nil)},
Expand Down Expand Up @@ -432,7 +432,7 @@ func TestRun(t *testing.T) {
},
},
expected: []Resource{NewResource("test", "Pod", "foo", "Pending",
&proto.ActionableErr{
proto.ActionableErr{
Message: "0/1 nodes are available: 1 node(s) had taint {key: value}, that the pod didn't tolerate",
ErrCode: proto.StatusCode_STATUSCHECK_FAILED_SCHEDULING,
}, nil)},
Expand Down
8 changes: 4 additions & 4 deletions pkg/diag/validator/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type Resource struct {
name string
logs []string
status Status
ae *proto.ActionableErr
ae proto.ActionableErr
}

func (r Resource) Kind() string { return r.kind }
Expand All @@ -45,15 +45,15 @@ func (r Resource) String() string {
}
return fmt.Sprintf("%s:%s/%s", r.namespace, r.kind, r.name)
}
func (r Resource) ActionableError() *proto.ActionableErr {
func (r Resource) ActionableError() proto.ActionableErr {
return r.ae
}
func (r Resource) StatusUpdated(another Resource) bool {
return r.ae.ErrCode != another.ae.ErrCode
}

// NewResource creates new Resource of kind
func NewResource(namespace, kind, name string, status Status, ae *proto.ActionableErr, logs []string) Resource {
func NewResource(namespace, kind, name string, status Status, ae proto.ActionableErr, logs []string) Resource {
return Resource{namespace: namespace, kind: kind, name: name, status: status, ae: ae, logs: logs}
}

Expand All @@ -64,6 +64,6 @@ type objectWithMetadata interface {
}

// NewResourceFromObject creates new Resource with fields populated from object metadata.
func NewResourceFromObject(object objectWithMetadata, status Status, ae *proto.ActionableErr, logs []string) Resource {
func NewResourceFromObject(object objectWithMetadata, status Status, ae proto.ActionableErr, logs []string) Resource {
return NewResource(object.GetNamespace(), object.GetObjectKind().GroupVersionKind().Kind, object.GetName(), status, ae, logs)
}
7 changes: 4 additions & 3 deletions pkg/diag/validator/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/GoogleContainerTools/skaffold/proto"
"github.com/GoogleContainerTools/skaffold/testutil"
)

Expand All @@ -42,7 +43,7 @@ func TestNewResource(t *testing.T) {
Name: "foo",
},
},
expected: Resource{"default", "pod", "foo", nil, Status(""), nil},
expected: Resource{"default", "pod", "foo", nil, Status(""), proto.ActionableErr{}},
expectedName: "pod/foo",
},
{
Expand All @@ -54,13 +55,13 @@ func TestNewResource(t *testing.T) {
Name: "bar",
},
},
expected: Resource{"test", "pod", "bar", nil, Status(""), nil},
expected: Resource{"test", "pod", "bar", nil, Status(""), proto.ActionableErr{}},
expectedName: "test:pod/bar",
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
actual := NewResourceFromObject(test.resource, Status(""), nil, nil)
actual := NewResourceFromObject(test.resource, Status(""), proto.ActionableErr{}, nil)
t.CheckDeepEqual(test.expected, actual, cmp.AllowUnexported(Resource{}))
t.CheckDeepEqual(test.expectedName, actual.String(), cmp.AllowUnexported(Resource{}))
})
Expand Down
16 changes: 8 additions & 8 deletions pkg/skaffold/deploy/resource/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (d *Deployment) Deadline() time.Duration {
return d.deadline
}

func (d *Deployment) UpdateStatus(ae *proto.ActionableErr) {
func (d *Deployment) UpdateStatus(ae proto.ActionableErr) {
updated := newStatus(ae)
if d.status.Equal(updated) {
d.status.changed = false
Expand All @@ -81,7 +81,7 @@ func NewDeployment(name string, ns string, deadline time.Duration) *Deployment {
name: name,
namespace: ns,
rType: deploymentType,
status: newStatus(&proto.ActionableErr{}),
status: newStatus(proto.ActionableErr{}),
deadline: deadline,
podValidator: diag.New(nil),
}
Expand Down Expand Up @@ -174,30 +174,30 @@ func (d *Deployment) cleanupStatus(msg string) string {
// $kubectl logs testPod -f
// 2020/06/18 17:28:31 service is running
// Killed: 9
func parseKubectlRolloutError(details string, err error) *proto.ActionableErr {
func parseKubectlRolloutError(details string, err error) proto.ActionableErr {
switch {
case err == nil && strings.Contains(details, rollOutSuccess):
return &proto.ActionableErr{
return proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_SUCCESS,
Message: details,
}
case err == nil:
return &proto.ActionableErr{
return proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_DEPLOYMENT_ROLLOUT_PENDING,
Message: details,
}
case strings.Contains(err.Error(), connectionErrMsg):
return &proto.ActionableErr{
return proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_KUBECTL_CONNECTION_ERR,
Message: MsgKubectlConnection,
}
case strings.Contains(err.Error(), killedErrMsg):
return &proto.ActionableErr{
return proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_KUBECTL_PID_KILLED,
Message: msgKubectlKilled,
}
default:
return &proto.ActionableErr{
return proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_UNKNOWN,
Message: err.Error(),
}
Expand Down
18 changes: 9 additions & 9 deletions pkg/skaffold/deploy/resource/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,36 +105,36 @@ func TestParseKubectlError(t *testing.T) {
description string
details string
err error
expectedAe *proto.ActionableErr
expectedAe proto.ActionableErr
}{
{
description: "rollout status connection error",
err: errors.New("Unable to connect to the server"),
expectedAe: &proto.ActionableErr{
expectedAe: proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_KUBECTL_CONNECTION_ERR,
Message: MsgKubectlConnection,
},
},
{
description: "rollout status kubectl command killed",
err: errors.New("signal: killed"),
expectedAe: &proto.ActionableErr{
expectedAe: proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_KUBECTL_PID_KILLED,
Message: msgKubectlKilled,
},
},
{
description: "rollout status random error",
err: errors.New("deployment test not found"),
expectedAe: &proto.ActionableErr{
expectedAe: proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_UNKNOWN,
Message: "deployment test not found",
},
},
{
description: "rollout status nil error",
details: "successfully rolled out",
expectedAe: &proto.ActionableErr{
expectedAe: proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_SUCCESS,
Message: "successfully rolled out",
},
Expand Down Expand Up @@ -195,17 +195,17 @@ func TestIsErrAndNotRetriable(t *testing.T) {
func TestReportSinceLastUpdated(t *testing.T) {
var tests = []struct {
description string
ae *proto.ActionableErr
ae proto.ActionableErr
expected string
}{
{
description: "updating an error status",
ae: &proto.ActionableErr{Message: "cannot pull image"},
ae: proto.ActionableErr{Message: "cannot pull image"},
expected: " - test-ns:deployment/test: cannot pull image",
},
{
description: "updating a non error status",
ae: &proto.ActionableErr{Message: "waiting for container"},
ae: proto.ActionableErr{Message: "waiting for container"},
expected: " - test-ns:deployment/test: waiting for container",
},
}
Expand Down Expand Up @@ -251,7 +251,7 @@ func TestReportSinceLastUpdatedMultipleTimes(t *testing.T) {
var actual string
for i, status := range test.statuses {
// update to same status
dep.UpdateStatus(&proto.ActionableErr{
dep.UpdateStatus(proto.ActionableErr{
ErrCode: proto.StatusCode_STATUSCHECK_DEPLOYMENT_ROLLOUT_PENDING,
Message: status,
})
Expand Down
Loading

0 comments on commit 270899b

Please sign in to comment.