v8 Breaking Changes Discussion

[taylorsilva]

Hey folks, there are some changes that are motivating me to make the next release a major bump to v8. None of them are new features, they're more along the lines of updates to existing functionality, refactors, and just paying down tech debt.

Please share your thoughts below about the changes proposed here. Feel free to suggest other breaking changes as well that you think may improve things for users.

Behavioural Changes

• Change the default value for put.inputs to detect. Current default is all
  • Why? all results in all artifacts from your build being mounted into your put step. This is usually not what you want. You usually only need a few artifacts from your build and they're usually declared in the put.params, which is how detect finds the artifacts to mount into the put step. I see this change only being an issue for resources who's put/out steps expect artifacts to have a certain name and don't let users of their resource declare any put.params. None of the built-in resource-types do this.
• Change the default container runtime to containerd
  • Why? The speed of development is faster with containerd. We actually get contributions for our worker/runtime package. The Guardian runtime only just got support for cgroups v2 working this year while containerd has had it for years. There have been a bunch of issues where people say they can't start Concourse and the solution is "set the runtime to containerd". The OSS cluster has been using containerd for years now without issue. The runtime feels VERY stable at this point and I feel confident setting it as the default runtime for Concourse moving forward.
• Enable the across step by default
  • It's not experimental anymore and we want to encourage more people to use it.
  • Deprecates the feature flag
• Enable the instance pipelines by default
  • Update docs: https://concourse-ci.org/set-pipeline-step.html#schema.set-pipeline.instance_vars
  • Deprecates the feature flag
• Enable redact secrets by default
  • Update docs: https://concourse-ci.org/creds-redacting.html
  • Deprecates the feature flag

I'll add to this list if I think of more behavioural stuff to change, or if folks suggest any.

Large Database Migrations

Large database migrations have been the reason for the last two majors of Concourse (v6 and v7). These are also the main motivation for doing a major bump.

• Switch use of md5 hashing to sha256 #9165 - Depending on database size this migration will takes tens of minutes. Operators will need to schedule significant downtime for this upgrade
• Migration for updating int to bigint takes long time for large Concourse deployments #8499 - I need to dig into this one more as I don't have context for it. I'd put this in the tech debt category for problems to solve. It will also likely result in a long migration that takes tens of minutes.

Actually Breaking

• We're migrating the resource-types and the concourse/concourse images to use Chainguard's wolfi-base. Functionality-wise this won't break anything in your pipelines. If users had some kind of tests or scanners that depended on stuff in the previous Paketo-based images then things might break in those systems external to Concourse. I don't expect this to be an issue though. This landed in v7.14
  • Why? We want to deliver ARM64 versions of the Concourse binary and image. That means every container image we release needs to be in AMD64 and ARM64 formats. The Paketo images only release AMD64 images. Therefore we need to migrate off of to something else. wolfi-base seems like a good alternative and should even result in smaller image sizes.
• We noticed that https://github.com/go-yaml/yaml (gopkg.in/yaml.v3) is now unmaintained. This might be a good time to move to a new library and figure out what breaks while doing it and how to ease the transition. That said, the library itself has no dependencies (it's go.mod has a dependency that's used for testing) and currently has no CVE's. It hasn't had a CVE for quite a few years now too. Regardless, we should explore alternatives.
  • Some alternatives to explore:
    • https://pkg.go.dev/sigs.k8s.io/yaml
    • https://pkg.go.dev/github.com/goccy/go-yaml
• Remove job.build_logs_to_retain. Users have been told to use job.build_logs_retention for a while now, so I think we can remove this field now.
• Remove fly's old-style templated values that used {{ }} instead of the current (( )) style. The old-style isn't even documented. It was only possible to use that syntax if you were setting a pipeline with `fly.
  • fly: drop support for deprecated {{old_style}} var resolution #9194
• Return an error instead of a warning when an invalid identifier is detected, as proposed by the RFC. Users have had 5 years to transition their identifiers to meet these requirements. This has proven too hard to enforce, mostly due to secrets. Kicking the can down the road!
• Return an error when a resource emits an empty version {} - an empty version has no semantic meaning in Concourse and results in a blank rectangle in the web UI. This was never explicitly invalid in the past, but is moving forward. Resources can emit empty versions #9279

[scottillogical]

Looks good, thank you for your work on this.

I would want to confirm that the long database migrations don't cause concourse to fail health checks and go into a restart loop on k8s

[taylorsilva]

I don't believe it would. The database migrations are done in an initContainer in the web's Deployment manifest: https://github.com/concourse/concourse-chart/blob/612edbe0e701664bd4df66ce4f7579697b3e6e92/templates/web-deployment.yaml#L57-L58

From what I recall there is no timeout on the initContainers, they simply run to completion, as per the k8s docs: https://kubernetes.io/docs/concepts/workloads/pods/init-containers/

[taylorsilva]

Added another "Actually Breaking" item:

Remove job.build_logs_to_retain...

This field has been superseded by build_log_retention for years now.

[taylorsilva]

From #9026 (comment) it was found that fly supports templating vars with {{ }} syntax. It seems this is a VERY old Concourse thing and it's not even documented on the site. I didn't even know it was a thing!

The tests that ensure this still works already called them "old-style templated" values. I'm going to assume no one is using this so I think it's fine to remove it in this 8.x release.

[taylorsilva]

PR removing support for this old syntax: #9194

[analytically]

PR removing yamlpatch package to support this syntax: #9154

[taylorsilva]

The valid identifiers RFC was opened and implemented 5 years ago. The RFC initially proposed a one year transition time. Users have had 5 years to transition instead.

Because we've only warned instead of errored, users have still been bitten by creating things with invalid identifiers:

• Fly cannot delete Instanced Pipelines with Special Characters #8725
• Step set-pipeline can take invalid/problematic target pipeline name (in conjunction with instanced pipelines) #8475

From this discussion here: #8725 (comment)
I think now is the time to start enforcing valid identifiers. Concourse will return an error now instead of a warning if an invalid identifier is detected.

[cscanlin]

@taylorsilva We use concourse variables as pipeline names when calling the set_pipeline task or on task names

- set_pipeline: ((.:pipeline_config.pipeline_name))

Or

- task: get-((.:repo))-static-configs

Which gives warnings:
'((.:pipeline_config.pipeline_name))' is not a valid identifier: must start with a lowercase letter
'get-((.:repo))-static-configs' is not a valid identifier: illegal character '('

Can you confirm that these cases will still be allowed? (The values referenced by the variables do not contain illegal characters)

[taylorsilva]

TLDR; yes, both should still be allowed. PR here: #9202

The first case we ran into during PR review and I want to make sure that still works. I didn't think about the task name also having a similar usage. I'll probably play around with putting vars in a bunch of places now just to see where I get warnings. Thanks for raising this concern.

[taylorsilva]

@cscanlin The two examples you gave above with the task and set_pipeline steps, are those both run in across steps or own their own, outside across steps? I'm guessing inside across steps, I just want to be certain.

[cscanlin]

Yup, both inside of across steps 👍

[taylorsilva]

We ended up NOT changing this. See #9202 for details.

[taylorsilva]

v8 is coming out soon and all items have been resolved.