Skip to content

Commit

Permalink
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 4, 2019
1 parent 4fc8663 commit b720948
Show file tree
Hide file tree
Showing 35 changed files with 748 additions and 1,142 deletions.
9 changes: 5 additions & 4 deletions Gopkg.lock

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

4 changes: 2 additions & 2 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ required = [

[[override]]
name = "github.com/knative/pkg"
# HEAD as of 2019-03-21
revision = "60fdcbcabd2faeb34328d8b2725dc76c59189453"
# HEAD as of 2019-04-02
revision = "281cda84ceb316a70c2eafc5b3250a4b72c2d468"

[[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
4 changes: 2 additions & 2 deletions pkg/logs/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"time"

buildv1alpha1 "github.com/knative/build/pkg/client/clientset/versioned/typed/build/v1alpha1"
duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
"github.com/knative/pkg/apis"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
Expand Down Expand Up @@ -227,7 +227,7 @@ func podName(cfg *rest.Config, out io.Writer, buildName, namespace string) (stri
return cluster.PodName, nil
}

condition := b.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
condition := b.Status.GetCondition(apis.ConditionSucceeded)
if condition.IsFalse() {
return "", fmt.Errorf("build failed for reason: %s and msg: %s", condition.Reason, condition.Message)
}
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 b720948

Please sign in to comment.