Switch the default for "embedded-status" feature flag to "minimal" on or after Jan 25, 2023

[abayer]

The deprecations list entry for PipelineRun.Status.TaskRuns and PipelineRun.Status.Runs has Jan 25, 2023 as the earliest date for removing those fields. I'd like to propose that we switch the default to minimal on Jan 25, 2023, and do the actual removal later.

Technically, we could switch the default now and actually remove PipelineRun.Status.TaskRuns, PipelineRun.Status.Runs, and the embedded-status flag (more specifically, the full and both options for the flag) on or after Jan 25, 2023, without violating the deprecation/removal policy, but I feel like changing the default to minimal only on/after the deprecation/removal date is more in the spirit of the policy.

Additionally, when we remove the full and both options, we may went to keep the PipelineRun.Status fields in v1beta1 for the rest of v1beta1s lifetime - once we remove the full/both options, those fields will no longer get written to by new PipelineRuns, and we can remove all of the internal code from the reconciler for handling the PipelineRun.Status fields, but any PipelineRun created before the switch to minimal by default or before the removal of full/both where someone manually sets embedded-status to one of those values which still exists in etcd would run into problems when, for example, getting deleted (based on past experiences I've had with fields I deleted). It feels like it'll make the most sense to just make those fields read-only and never actually remove them.

Thoughts, @tektoncd/core-maintainers, @JeromeJu?

[JeromeJu]

Thanks Andrew for creating this issue. Marking PipelineRun.Status.TaskRuns and PipelineRun.Status.Runs as read-only instead of entirely removing them sounds reasonable.

someone manually sets embedded-status to one of those values which still exists in etcd would run into problems when
I think we shall also include the case where the embedded-status is flipped back and forth. (added as my TODO)

/assign

[jerop]

related to #4954

[lbernick]

Technically, we could switch the default now and actually remove PipelineRun.Status.TaskRuns, PipelineRun.Status.Runs, and the embedded-status flag (more specifically, the full and both options for the flag) on or after Jan 25, 2023, without violating the deprecation/removal policy, but I feel like changing the default to minimal only on/after the deprecation/removal date is more in the spirit of the policy.

Based on this section of TEP-0033 I agree that we should wait until Jan 23 to change the default value of the flag.

Additionally, when we remove the full and both options, we may went to keep the PipelineRun.Status fields in v1beta1 for the rest of v1beta1s lifetime - once we remove the full/both options, those fields will no longer get written to by new PipelineRuns, and we can remove all of the internal code from the reconciler for handling the PipelineRun.Status fields, but any PipelineRun created before the switch to minimal by default or before the removal of full/both where someone manually sets embedded-status to one of those values which still exists in etcd would run into problems when, for example, getting deleted (based on past experiences I've had with fields I deleted).

This should be fixed now by #5610. Do you know of any other issues that might be caused by keeping these fields around? If not, I think we should remove them. If so, probably OK to keep as read only, but I'd prefer removing them as we don't really have a mechanism for enforcing read-only (other than reviewer discipline) which I'd be concerned can lead to bugs.

[abayer]

Do you know of any other issues that might be caused by keeping these fields around? If not, I think we should remove them. If so, probably OK to keep as read only, but I'd prefer removing them as we don't really have a mechanism for enforcing read-only (other than reviewer discipline) which I'd be concerned can lead to bugs

The most obvious case is in-flight PipelineRuns - if we delete the fields right as we force everything to minimal behavior, then anything currently running in a cluster with full or both previously configured will barf on reconcile. We've been bitten by issues relating to deleted fields a number of times in the past - the "ignore validation on delete" helps, but I'm not confident that covers all potential issues. I am, admittedly, particularly conservative about changes that could break existing users, so I may be more cautious on this front than is strictly necessary. =)

And re: "enforcing read-only"...well, we only write to those fields in code that is only reached when embedded-status is set to both or full, and we'll obviously be removing that code when we get rid of support for both or full, so it should be safe to assume nothing will write to them?

[lbernick]

The most obvious case is in-flight PipelineRuns - if we delete the fields right as we force everything to minimal behavior, then anything currently running in a cluster with full or both previously configured will barf on reconcile.

This seems avoidable by removing them at least one release after removing the flag. I guess many people don't upgrade one release at a time, but this could be mitigated by release notes?

We've been bitten by issues relating to deleted fields a number of times in the past - the "ignore validation on delete" helps, but I'm not confident that covers all potential issues.

Are there any issues you can link to, or can you be more specific about what issues you're concerned about?

And re: "enforcing read-only"...well, we only write to those fields in code that is only reached when embedded-status is set to both or full, and we'll obviously be removing that code when we get rid of support for both or full, so it should be safe to assume nothing will write to them?

fair enough, this would probably be fine

[abayer]

This seems avoidable by removing them at least one release after removing the flag. I guess many people don't upgrade one release at a time, but this could be mitigated by release notes?
Yeah, I'd say if we go like 3+ months from removing the flag to removing the fields, that'd be acceptable.

Are there any issues you can link to, or can you be more specific about what issues you're concerned about?
The only ones I can clearly remember were deletion-related, so I may just be overly nervous. =)