Skip to content

Commit

Permalink
Cowardly refuse to attempt builds with a platform api > 0.2
Browse files Browse the repository at this point in the history
- Save build pod creation error in build status
  • Loading branch information
matthewmcnew committed Jan 24, 2020
1 parent 3fcd05a commit d4cc9f2
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 13 deletions.
18 changes: 18 additions & 0 deletions pkg/apis/build/v1alpha1/build_lifecycle.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package v1alpha1

import (
corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func (bs *BuildStatus) Error(err error) {
bs.Conditions = corev1alpha1.Conditions{
{
LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
Type: corev1alpha1.ConditionSucceeded,
Status: corev1.ConditionFalse,
Message: err.Error(),
},
}
}
9 changes: 9 additions & 0 deletions pkg/apis/build/v1alpha1/build_pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -82,6 +83,10 @@ var (
)

func (b *Build) BuildPod(config BuildPodImages, secrets []corev1.Secret, bc BuildPodBuilderConfig) (*corev1.Pod, error) {
if bc.unsupported() {
return nil, errors.Errorf("incompatible builder platform API version: %s", bc.PlatformAPI)
}

if b.rebasable(bc.StackID) {
return b.rebasePod(secrets, config, bc)
}
Expand Down Expand Up @@ -517,6 +522,10 @@ func (bc *BuildPodBuilderConfig) legacy() bool {
return bc.PlatformAPI == "0.1"
}

func (bc *BuildPodBuilderConfig) unsupported() bool {
return bc.PlatformAPI != "0.1" && bc.PlatformAPI != "0.2"
}

func builderSecretVolume(bbs BuildBuilderSpec) corev1.Volume {
if len(bbs.ImagePullSecrets) > 0 {
return corev1.Volume{
Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/build/v1alpha1/build_pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,15 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
})
})

when("0.3+ platform api", func() {
buildPodBuilderConfig.PlatformAPI = "0.3"

it("returns an error", func() {
_, err := build.BuildPod(config, secrets, buildPodBuilderConfig)
require.Error(t, err, "incompatible builder platform API version 0.3")
})
})

when("creating a rebase pod", func() {
it("creates a pod just to rebase", func() {
build.Annotations = map[string]string{v1alpha1.BuildReasonAnnotation: v1alpha1.BuildReasonStack, "some/annotation": "to-pass-through"}
Expand Down
31 changes: 31 additions & 0 deletions pkg/apis/build/v1alpha1/build_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
package v1alpha1

import (
"errors"
"testing"

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1"
)

func TestRebaseable(t *testing.T) {
Expand Down Expand Up @@ -44,3 +49,29 @@ func TestRebaseable(t *testing.T) {
require.True(t, build.rebasable("matching.stack"))

}

func TestBuildLifecycle(t *testing.T) {
build := &Build{
ObjectMeta: metav1.ObjectMeta{
Name: "test-name",
},
}
build.Status.Error(errors.New("error: display this error"))

require.True(t, equality.Semantic.DeepEqual(build, &Build{
ObjectMeta: metav1.ObjectMeta{
Name: "test-name",
},
Status: BuildStatus{
Status: corev1alpha1.Status{
Conditions: corev1alpha1.Conditions{
{
Type: corev1alpha1.ConditionSucceeded,
Status: corev1.ConditionFalse,
Message: "error: display this error",
},
},
},
},
}))
}
32 changes: 22 additions & 10 deletions pkg/reconciler/v1alpha1/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
build = build.DeepCopy()
build.SetDefaults(ctx)

reconcileErr := c.reconcile(build)
if reconcileErr != nil {
build.Status.Error(reconcileErr)
}

err = c.updateStatus(build)
if err != nil {
return err
}
return reconcileErr
}

func (c *Reconciler) reconcile(build *v1alpha1.Build) error {
if build.Finished() {
return nil
}
Expand All @@ -109,24 +122,22 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
build.Status.StepStates = stepStates(pod)
build.Status.StepsCompleted = stepCompleted(pod)
build.Status.Conditions = conditionForPod(pod)
build.Status.ObservedGeneration = build.Generation

return c.updateStatus(build)
return nil
}

func (c *Reconciler) reconcileBuildPod(build *v1alpha1.Build) (*corev1.Pod, error) {
pod, err := c.PodLister.Pods(build.Namespace).Get(build.PodName())
if err != nil && !k8s_errors.IsNotFound(err) {
return nil, err
} else if k8s_errors.IsNotFound(err) {
podConfig, err := c.PodGenerator.Generate(build)
if err != nil {
return nil, err
}
return c.K8sClient.CoreV1().Pods(build.Namespace).Create(podConfig)
} else if !k8s_errors.IsNotFound(err) {
return pod, nil
}

return pod, nil
podConfig, err := c.PodGenerator.Generate(build)
if err != nil {
return nil, controller.NewPermanentError(err)
}
return c.K8sClient.CoreV1().Pods(build.Namespace).Create(podConfig)
}

func conditionForPod(pod *corev1.Pod) corev1alpha1.Conditions {
Expand Down Expand Up @@ -178,6 +189,7 @@ func stepCompleted(pod *corev1.Pod) []string {
}

func (c *Reconciler) updateStatus(desired *v1alpha1.Build) error {
desired.Status.ObservedGeneration = desired.Generation
original, err := c.Lister.Builds(desired.Namespace).Get(desired.Name)
if err != nil {
return err
Expand Down
43 changes: 40 additions & 3 deletions pkg/reconciler/v1alpha1/build/build_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package build_test

import (
"errors"
"testing"
"time"

Expand Down Expand Up @@ -45,10 +46,9 @@ func testBuildReconciler(t *testing.T, when spec.G, it spec.S) {

var (
fakeMetadataRetriever = &buildfakes.FakeMetadataRetriever{}
podGenerator = &testPodGenerator{}
)

podGenerator := &testPodGenerator{}

rt := testhelpers.ReconcilerTester(t,
func(t *testing.T, row *rtesting.TableRow) (reconciler controller.Reconciler, lists rtesting.ActionRecorderList, list rtesting.EventList, reporter *rtesting.FakeStatsReporter) {
listers := testhelpers.NewListers(row.Objects)
Expand Down Expand Up @@ -250,6 +250,38 @@ func testBuildReconciler(t *testing.T, when spec.G, it spec.S) {
})
})

it("saves error creating build to status", func() {
podGenerator.returnErr = errors.New("display me in the status")

rt.Test(rtesting.TableRow{
Key: key,
Objects: []runtime.Object{
build,
},
WantErr: true,
WantStatusUpdates: []clientgotesting.UpdateActionImpl{
{
Object: &v1alpha1.Build{
ObjectMeta: build.ObjectMeta,
Spec: build.Spec,
Status: v1alpha1.BuildStatus{
Status: corev1alpha1.Status{
ObservedGeneration: 1,
Conditions: corev1alpha1.Conditions{
{
Type: corev1alpha1.ConditionSucceeded,
Status: corev1.ConditionFalse,
Message: "display me in the status",
},
},
},
},
},
},
},
})
})

when("pod executing", func() {
it("updates the status with the status of the pod", func() {
pod, err := podGenerator.Generate(build)
Expand Down Expand Up @@ -715,9 +747,14 @@ func testBuildReconciler(t *testing.T, when spec.G, it spec.S) {
}

type testPodGenerator struct {
returnErr error
}

func (testPodGenerator) Generate(build buildpod.BuildPodable) (*corev1.Pod, error) {
func (tpg testPodGenerator) Generate(build buildpod.BuildPodable) (*corev1.Pod, error) {
if tpg.returnErr != nil {
return nil, tpg.returnErr
}

return &corev1.Pod{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Expand Down

0 comments on commit d4cc9f2

Please sign in to comment.