Skip to content

Commit

Permalink
Debugging test failures after swapping to minimal
Browse files Browse the repository at this point in the history
  • Loading branch information
JeromeJu committed Jan 12, 2023
1 parent 0f42ebf commit 189a3b0
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 34 deletions.
2 changes: 1 addition & 1 deletion pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const (
// DefaultSendCloudEventsForRuns is the default value for "send-cloudevents-for-runs".
DefaultSendCloudEventsForRuns = false
// DefaultEmbeddedStatus is the default value for "embedded-status".
DefaultEmbeddedStatus = FullEmbeddedStatus
DefaultEmbeddedStatus = MinimalEmbeddedStatus
// DefaultEnableSpire is the default value for "enable-spire".
DefaultEnableSpire = false
// DefaultResourceVerificationMode is the default value for "resource-verification-mode".
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
{
expectedConfig: &config.FeatureFlags{
EnableAPIFields: "stable",
EmbeddedStatus: "full",
EmbeddedStatus: config.DefaultEmbeddedStatus,
EnableCustomTasks: config.DefaultEnableCustomTasks,
EnableSpire: true,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
Expand All @@ -158,7 +158,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
{
expectedConfig: &config.FeatureFlags{
EnableAPIFields: "stable",
EmbeddedStatus: "full",
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness,
Expand Down
1 change: 0 additions & 1 deletion pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,6 @@ func TestPipelineRunConversionFromDeprecated(t *testing.T) {
}
}

// Assumptions: The `embedded-status` is defaulted as `Full` as in config.DefaultEmbeddedStatus:
func TestPipelineRunConversionEmbeddedStatusRoundTrip(t *testing.T) {
taskRuns["tr-0"] = trs
runs["r-0"] = rrs
Expand Down
46 changes: 23 additions & 23 deletions pkg/reconciler/pipelinerun/cancel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,17 @@ func TestCancelPipelineRun(t *testing.T) {
customRuns []*v1beta1.CustomRun
wantErr bool
}{{
name: "no-resolved-taskrun",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "no-resolved-taskrun-with-full",
embeddedStatus: config.FullEmbeddedStatus,
pipelineRun: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
Spec: v1beta1.PipelineRunSpec{
Status: v1beta1.PipelineRunSpecStatusCancelled,
},
},
}, {
name: "one-taskrun",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "one-taskrun-with-full",
embeddedStatus: config.FullEmbeddedStatus,
pipelineRun: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
Spec: v1beta1.PipelineRunSpec{
Expand All @@ -72,8 +72,8 @@ func TestCancelPipelineRun(t *testing.T) {
{ObjectMeta: metav1.ObjectMeta{Name: "t1"}},
},
}, {
name: "multiple-taskruns-one-missing",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "multiple-taskruns-one-missing-with-full",
embeddedStatus: config.FullEmbeddedStatus,
pipelineRun: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
Spec: v1beta1.PipelineRunSpec{
Expand All @@ -90,8 +90,8 @@ func TestCancelPipelineRun(t *testing.T) {
{ObjectMeta: metav1.ObjectMeta{Name: "t2"}},
},
}, {
name: "multiple-taskruns",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "multiple-taskruns-with-full",
embeddedStatus: config.FullEmbeddedStatus,
pipelineRun: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
Spec: v1beta1.PipelineRunSpec{
Expand All @@ -109,8 +109,8 @@ func TestCancelPipelineRun(t *testing.T) {
{ObjectMeta: metav1.ObjectMeta{Name: "t2"}},
},
}, {
name: "multiple-runs",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "multiple-runs-with-full",
embeddedStatus: config.FullEmbeddedStatus,
pipelineRun: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
Spec: v1beta1.PipelineRunSpec{
Expand All @@ -128,8 +128,8 @@ func TestCancelPipelineRun(t *testing.T) {
{ObjectMeta: metav1.ObjectMeta{Name: "t2"}},
},
}, {
name: "multiple-runs-one-missing",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "multiple-runs-one-missing-with-full",
embeddedStatus: config.FullEmbeddedStatus,
pipelineRun: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
Spec: v1beta1.PipelineRunSpec{
Expand Down Expand Up @@ -227,7 +227,7 @@ func TestCancelPipelineRun(t *testing.T) {
},
}, {
name: "child-references-with-minimal",
embeddedStatus: config.MinimalEmbeddedStatus,
embeddedStatus: config.DefaultEmbeddedStatus,
pipelineRun: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
Spec: v1beta1.PipelineRunSpec{
Expand Down Expand Up @@ -268,7 +268,7 @@ func TestCancelPipelineRun(t *testing.T) {
},
}, {
name: "child-references-with-minimal-some-missing",
embeddedStatus: config.MinimalEmbeddedStatus,
embeddedStatus: config.DefaultEmbeddedStatus,
pipelineRun: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
Spec: v1beta1.PipelineRunSpec{
Expand Down Expand Up @@ -459,8 +459,8 @@ func TestGetChildObjectsFromPRStatusForTaskNames(t *testing.T) {
hasError bool
}{
{
name: "single taskrun, default embedded",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "single taskrun, full embedded",
embeddedStatus: config.FullEmbeddedStatus,
prStatus: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
TaskRuns: map[string]*v1beta1.PipelineRunTaskRunStatus{
"t1": {PipelineTaskName: "task-1"},
Expand All @@ -471,8 +471,8 @@ func TestGetChildObjectsFromPRStatusForTaskNames(t *testing.T) {
expectedCustomRunNames: nil,
hasError: false,
}, {
name: "single run, default embedded",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "single run, full embedded",
embeddedStatus: config.FullEmbeddedStatus,
prStatus: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
Runs: map[string]*v1beta1.PipelineRunRunStatus{
"r1": {PipelineTaskName: "run-1"},
Expand All @@ -482,8 +482,8 @@ func TestGetChildObjectsFromPRStatusForTaskNames(t *testing.T) {
expectedRunNames: []string{"r1"},
hasError: false,
}, {
name: "taskrun and run, default embedded",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "taskrun and run, full embedded",
embeddedStatus: config.FullEmbeddedStatus,
prStatus: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
TaskRuns: map[string]*v1beta1.PipelineRunTaskRunStatus{
"t1": {PipelineTaskName: "task-1"},
Expand All @@ -496,8 +496,8 @@ func TestGetChildObjectsFromPRStatusForTaskNames(t *testing.T) {
expectedRunNames: []string{"r1"},
hasError: false,
}, {
name: "taskrun and run, default embedded, just want taskrun",
embeddedStatus: config.DefaultEmbeddedStatus,
name: "taskrun and run, full embedded, just want taskrun",
embeddedStatus: config.FullEmbeddedStatus,
prStatus: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
TaskRuns: map[string]*v1beta1.PipelineRunTaskRunStatus{
"t1": {PipelineTaskName: "task-1"},
Expand Down Expand Up @@ -549,7 +549,7 @@ func TestGetChildObjectsFromPRStatusForTaskNames(t *testing.T) {
expectedRunNames: []string{"r1"},
hasError: false,
}, {
name: "minimal embedded",
name: "default minimal embedded",
embeddedStatus: config.MinimalEmbeddedStatus,
prStatus: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
TaskRuns: map[string]*v1beta1.PipelineRunTaskRunStatus{
Expand Down
17 changes: 10 additions & 7 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ var (
func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) pkgreconciler.Event {
logger := logging.FromContext(ctx)
ctx = cloudevent.ToContext(ctx, c.cloudEventClient)
cfg := config.FromContextOrDefaults(ctx)

// Read the initial condition
before := pr.Status.GetCondition(apis.ConditionSucceeded)
Expand Down Expand Up @@ -204,13 +205,15 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun)
logger.Errorf("Failed to delete StatefulSet for PipelineRun %s: %v", pr.Name, err)
return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err)
}
if err := c.updateTaskRunsStatusDirectly(pr); err != nil {
logger.Errorf("Failed to update TaskRun status for PipelineRun %s: %v", pr.Name, err)
return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err)
}
if err := c.updateRunsStatusDirectly(pr); err != nil {
logger.Errorf("Failed to update Run status for PipelineRun %s: %v", pr.Name, err)
return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err)
if cfg.FeatureFlags.EmbeddedStatus == config.FullEmbeddedStatus || cfg.FeatureFlags.EmbeddedStatus == config.BothEmbeddedStatus {
if err := c.updateTaskRunsStatusDirectly(pr); err != nil {
logger.Errorf("Failed to update TaskRun status for PipelineRun %s: %v", pr.Name, err)
return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err)
}
if err := c.updateRunsStatusDirectly(pr); err != nil {
logger.Errorf("Failed to update Run status for PipelineRun %s: %v", pr.Name, err)
return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err)
}
}
return c.finishReconcileUpdateEmitEvents(ctx, pr, before, nil)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1836,6 +1836,7 @@ status:
}
}

// TODO
func TestReconcileForCustomTaskWithPipelineRunTimedOut(t *testing.T) {
names.TestingSeed()
// TestReconcileForCustomTaskWithPipelineRunTimedOut runs "Reconcile" on a
Expand Down

0 comments on commit 189a3b0

Please sign in to comment.