Skip to content

Commit

Permalink
[WIP] Move to the v1beta1 Conditions implementation.
Browse files Browse the repository at this point in the history
See here for more context on what's changed (no breaking changes): knative/pkg#361
  • Loading branch information
mattmoor committed Apr 2, 2019
1 parent 8b955e3 commit 0ac580f
Show file tree
Hide file tree
Showing 32 changed files with 1,251 additions and 241 deletions.
8 changes: 6 additions & 2 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ required = [
[[override]]
name = "github.com/knative/pkg"
# HEAD as of 2019-03-21
revision = "60fdcbcabd2faeb34328d8b2725dc76c59189453"
#TODO(mattmoor): DO NOT SUBMIT
source = "github.com/mattmoor/pkg-1"
revision = "766353dc976aeab0178ef2ad39d6e6eff3e3072d"

[[override]]
name = "contrib.go.opencensus.io/exporter/stackdriver"
Expand Down
29 changes: 7 additions & 22 deletions pkg/apis/build/v1alpha1/build_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"

"github.com/knative/pkg/apis"
duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
duckv1beta1 "github.com/knative/pkg/apis/duck/v1beta1"
"github.com/knative/pkg/kmeta"
)

Expand Down Expand Up @@ -238,7 +238,7 @@ const (

// BuildStatus is the status for a Build resource
type BuildStatus struct {
duckv1alpha1.Status `json:",inline"`
duckv1beta1.Status `json:",inline"`

// +optional
Builder BuildProvider `json:"builder,omitempty"`
Expand Down Expand Up @@ -268,9 +268,6 @@ type BuildStatus struct {
StepsCompleted []string `json:"stepsCompleted",omitempty`
}

// Check that BuildStatus may have its conditions managed.
var _ duckv1alpha1.ConditionsAccessor = (*BuildStatus)(nil)

// ClusterSpec provides information about the on-cluster build, if applicable.
type ClusterSpec struct {
// Namespace is the namespace in which the pod is running.
Expand All @@ -290,11 +287,11 @@ type GoogleSpec struct {
//
// If the build is ongoing, its status will be Unknown. If it fails, its status
// will be False.
const BuildSucceeded = duckv1alpha1.ConditionSucceeded
const BuildSucceeded = apis.ConditionSucceeded

const BuildCancelled duckv1alpha1.ConditionType = "Cancelled"
const BuildCancelled apis.ConditionType = "Cancelled"

var buildCondSet = duckv1alpha1.NewBatchConditionSet()
var buildCondSet = apis.NewBatchConditionSet()

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

Expand All @@ -308,30 +305,18 @@ type BuildList struct {
}

// GetCondition returns the Condition matching the given type.
func (bs *BuildStatus) GetCondition(t duckv1alpha1.ConditionType) *duckv1alpha1.Condition {
func (bs *BuildStatus) GetCondition(t apis.ConditionType) *apis.Condition {
return buildCondSet.Manage(bs).GetCondition(t)
}

// SetCondition sets the condition, unsetting previous conditions with the same
// type as necessary.
func (bs *BuildStatus) SetCondition(newCond *duckv1alpha1.Condition) {
func (bs *BuildStatus) SetCondition(newCond *apis.Condition) {
if newCond != nil {
buildCondSet.Manage(bs).SetCondition(*newCond)
}
}

// GetConditions returns the Conditions array. This enables generic handling of
// conditions by implementing the duckv1alpha1.Conditions interface.
func (bs *BuildStatus) GetConditions() duckv1alpha1.Conditions {
return bs.Conditions
}

// SetConditions sets the Conditions array. This enables generic handling of
// conditions by implementing the duckv1alpha1.Conditions interface.
func (bs *BuildStatus) SetConditions(conditions duckv1alpha1.Conditions) {
bs.Conditions = conditions
}

func (b *Build) GetGroupVersionKind() schema.GroupVersionKind {
return SchemeGroupVersion.WithKind("Build")
}
20 changes: 10 additions & 10 deletions pkg/apis/build/v1alpha1/build_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,22 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/knative/pkg/apis"
"github.com/knative/pkg/apis/duck"
duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
duckv1beta1 "github.com/knative/pkg/apis/duck/v1beta1"
)

func TestBuildImplementsConditions(t *testing.T) {
if err := duck.VerifyType(&Build{}, &duckv1alpha1.Conditions{}); err != nil {
if err := duck.VerifyType(&Build{}, &duckv1beta1.Conditions{}); err != nil {
t.Errorf("Expect Build to implement duck verify type: err %#v", err)
}
}

func TestBuildConditions(t *testing.T) {
b := &Build{}
foo := &duckv1alpha1.Condition{
foo := &apis.Condition{
Type: "Foo",
Status: "True",
}
bar := &duckv1alpha1.Condition{
bar := &apis.Condition{
Type: "Bar",
Status: "True",
}
Expand All @@ -49,20 +49,20 @@ func TestBuildConditions(t *testing.T) {
// Add a new condition.
b.Status.SetCondition(foo)

want := duckv1alpha1.Conditions([]duckv1alpha1.Condition{*foo})
if cmp.Diff(b.Status.GetConditions(), want, ignoreVolatileTime) != "" {
t.Errorf("Unexpected build condition type; want %v got %v", want, b.Status.GetConditions())
want := apis.Conditions{*foo}
if diff := cmp.Diff(b.Status.GetConditions(), want, ignoreVolatileTime); diff != "" {
t.Errorf("Unexpected build condition type; %s", diff)
}

fooStatus := b.Status.GetCondition(foo.Type)
if cmp.Diff(fooStatus, foo, ignoreVolatileTime) != "" {
t.Errorf("Unexpected build condition type; want %v got %v", fooStatus, foo)
if diff := cmp.Diff(fooStatus, foo, ignoreVolatileTime); diff != "" {
t.Errorf("Unexpected build condition type; %s", diff)
}

// Add a second condition.
b.Status.SetCondition(bar)

want = duckv1alpha1.Conditions([]duckv1alpha1.Condition{*bar, *foo})
want = apis.Conditions{*bar, *foo}

if d := cmp.Diff(b.Status.GetConditions(), want, ignoreVolatileTime); d != "" {
t.Fatalf("Unexpected build condition type; want %v got %v; diff %s", want, b.Status.GetConditions(), d)
Expand Down
10 changes: 5 additions & 5 deletions pkg/reconciler/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
listers "github.com/knative/build/pkg/client/listers/build/v1alpha1"
"github.com/knative/build/pkg/reconciler"
"github.com/knative/build/pkg/reconciler/build/resources"
duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
"github.com/knative/pkg/apis"
"github.com/knative/pkg/controller"
"github.com/knative/pkg/logging"
"github.com/knative/pkg/logging/logkey"
Expand Down Expand Up @@ -196,7 +196,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
PodName: "",
},
}
build.Status.SetCondition(&duckv1alpha1.Condition{
build.Status.SetCondition(&apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionFalse,
Reason: "BuildValidationFailed",
Expand All @@ -210,7 +210,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {

p, err = c.startPodForBuild(build)
if err != nil {
build.Status.SetCondition(&duckv1alpha1.Condition{
build.Status.SetCondition(&apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionFalse,
Reason: "BuildExecuteFailed",
Expand Down Expand Up @@ -314,7 +314,7 @@ func isCancelled(buildSpec v1alpha1.BuildSpec) bool {

func (c *Reconciler) cancelBuild(build *v1alpha1.Build, logger *zap.SugaredLogger) error {
logger.Warnf("Build has been cancelled: %v", build.Name)
build.Status.SetCondition(&duckv1alpha1.Condition{
build.Status.SetCondition(&apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionFalse,
Reason: "BuildCancelled",
Expand Down Expand Up @@ -359,7 +359,7 @@ func (c *Reconciler) checkTimeout(build *v1alpha1.Build) error {
}

timeoutMsg := fmt.Sprintf("Build %q failed to finish within %q", build.Name, timeout.String())
build.Status.SetCondition(&duckv1alpha1.Condition{
build.Status.SetCondition(&apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionFalse,
Reason: "BuildTimeout",
Expand Down
33 changes: 16 additions & 17 deletions pkg/reconciler/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/knative/build/pkg/client/clientset/versioned/fake"
informers "github.com/knative/build/pkg/client/informers/externalversions"
"github.com/knative/pkg/apis"
duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
"github.com/knative/pkg/controller"
"go.uber.org/zap"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -220,8 +219,8 @@ func TestBuildWithMissingServiceAccount(t *testing.T) {
t.Errorf("error fetching build: %v", err)
}
// Check that build has the expected status.
gotCondition := b.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
if d := cmp.Diff(gotCondition, &duckv1alpha1.Condition{
gotCondition := b.Status.GetCondition(apis.ConditionSucceeded)
if d := cmp.Diff(gotCondition, &apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionFalse,
Reason: "BuildValidationFailed",
Expand Down Expand Up @@ -268,8 +267,8 @@ func TestBuildWithMissingSecret(t *testing.T) {
t.Errorf("error fetching build: %v", err)
}
// Check that build has the expected status.
gotCondition := b.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
if d := cmp.Diff(gotCondition, &duckv1alpha1.Condition{
gotCondition := b.Status.GetCondition(apis.ConditionSucceeded)
if d := cmp.Diff(gotCondition, &apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionFalse,
Reason: "BuildValidationFailed",
Expand Down Expand Up @@ -316,8 +315,8 @@ func TestBuildWithNonExistentTemplates(t *testing.T) {
t.Errorf("error fetching build: %v", err)
}
// Check that build has the expected status.
gotCondition := b.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
if d := cmp.Diff(gotCondition, &duckv1alpha1.Condition{
gotCondition := b.Status.GetCondition(apis.ConditionSucceeded)
if d := cmp.Diff(gotCondition, &apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionFalse,
Reason: "BuildValidationFailed",
Expand Down Expand Up @@ -390,18 +389,18 @@ func TestBasicFlows(t *testing.T) {
for _, c := range []struct {
desc string
podStatus corev1.PodStatus
wantCondition *duckv1alpha1.Condition
wantCondition *apis.Condition
}{{
desc: "success",
podStatus: corev1.PodStatus{Phase: corev1.PodSucceeded},
wantCondition: &duckv1alpha1.Condition{
wantCondition: &apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionTrue,
},
}, {
desc: "running",
podStatus: corev1.PodStatus{Phase: corev1.PodRunning},
wantCondition: &duckv1alpha1.Condition{
wantCondition: &apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionUnknown,
Reason: "Building",
Expand All @@ -412,7 +411,7 @@ func TestBasicFlows(t *testing.T) {
Phase: corev1.PodFailed,
Message: "boom",
},
wantCondition: &duckv1alpha1.Condition{
wantCondition: &apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionFalse,
Message: "boom",
Expand All @@ -432,7 +431,7 @@ func TestBasicFlows(t *testing.T) {
},
}},
},
wantCondition: &duckv1alpha1.Condition{
wantCondition: &apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionUnknown,
Reason: "Pending",
Expand Down Expand Up @@ -501,7 +500,7 @@ func TestBasicFlows(t *testing.T) {
}

// Check that build has the expected status.
gotCondition := b.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
gotCondition := b.Status.GetCondition(apis.ConditionSucceeded)
if d := cmp.Diff(gotCondition, c.wantCondition, ignoreVolatileTime); d != "" {
t.Errorf("Unexpected build status %s", d)
}
Expand Down Expand Up @@ -547,8 +546,8 @@ func TestTimeoutFlow(t *testing.T) {
if err != nil {
t.Errorf("error fetching build: %v", err)
}
if d := cmp.Diff(b.Status.GetCondition(duckv1alpha1.ConditionSucceeded), &duckv1alpha1.Condition{
Type: duckv1alpha1.ConditionSucceeded,
if d := cmp.Diff(b.Status.GetCondition(apis.ConditionSucceeded), &apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
Reason: "BuildTimeout",
Message: fmt.Sprintf("Build %q failed to finish within \"500ms\"", b.Name),
Expand Down Expand Up @@ -607,8 +606,8 @@ func TestCancelledFlow(t *testing.T) {
if err != nil {
t.Errorf("error fetching build: %v", err)
}
if d := cmp.Diff(b.Status.GetCondition(duckv1alpha1.ConditionSucceeded), &duckv1alpha1.Condition{
Type: duckv1alpha1.ConditionSucceeded,
if d := cmp.Diff(b.Status.GetCondition(apis.ConditionSucceeded), &apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
Reason: "BuildCancelled",
Message: fmt.Sprintf("Build %q was cancelled", b.Name),
Expand Down
9 changes: 4 additions & 5 deletions pkg/reconciler/build/resources/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import (
"github.com/knative/build/pkg/credentials/dockercreds"
"github.com/knative/build/pkg/credentials/gitcreds"
"github.com/knative/pkg/apis"
duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
)

const workspaceDir = "/workspace"
Expand Down Expand Up @@ -433,28 +432,28 @@ func BuildStatusFromPod(p *corev1.Pod, buildSpec v1alpha1.BuildSpec) v1alpha1.Bu

switch p.Status.Phase {
case corev1.PodRunning:
status.SetCondition(&duckv1alpha1.Condition{
status.SetCondition(&apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionUnknown,
Reason: "Building",
})
case corev1.PodFailed:
msg := getFailureMessage(p)
status.SetCondition(&duckv1alpha1.Condition{
status.SetCondition(&apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionFalse,
Message: msg,
})
case corev1.PodPending:
msg := getWaitingMessage(p)
status.SetCondition(&duckv1alpha1.Condition{
status.SetCondition(&apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionUnknown,
Reason: "Pending",
Message: msg,
})
case corev1.PodSucceeded:
status.SetCondition(&duckv1alpha1.Condition{
status.SetCondition(&apis.Condition{
Type: v1alpha1.BuildSucceeded,
Status: corev1.ConditionTrue,
})
Expand Down
Loading

0 comments on commit 0ac580f

Please sign in to comment.