iamkirkbater
commented
3 months ago This is great Chris! I'm super excited to test this.
Just a few notes from my read-through of your description: I absolutely appreciate the messaging to deprecate some functionality - however I'd like to consider this a breaking major version change with semver (so instead of tagging as v0.1 -> v0.2, we make this v0.1 -> v1.0) and I'm more OK with removing the functionality of most things without deprecation warnings when we go to publish this.
As I would consider this PR a release candidate - I'm more than happy to keep the deprecation warnings though to help with the testing transition, but upon release I'd much rather have these deprecation warnings and the functionality be actually removed and call it a breaking change, rather than having to plan to actually remove later.
Aside from that - I'm excited to get this pulled down and start testing this during my shift next week :D
rendhalver
clcollins
openshift-ci[bot]
This is a large PR, rewriting (or reimplementing, really) ocm-container in golang. The intent is to make this more testable, more extensible, to fix some bash-induced bugs, and finally, to allow us to make and publish a binary to more easily, reliably and securely onboard new users.
This rewrite is an (almost) 1-1 feature parity and backward-compatible replacement for the ocm-container.sh file, as well as a number of other related tweaks.
Opinionated changes
In a (very) few places, it's a little more opinionated and will possibly require a minor change in usage, but for the most part opinionated changes are displayed as deprecation messages, and we can decide in the future how and when to handle them. In most of these cases, it's to fix some ocm-container-isms that prevent us from utilizing some of the features of the container:
eg:
ocm-container CLUSTERIDis difficult to parse, and prevents us from easily using container-standard Entrypoint and CMD flags. This PR deprecates that and addsocm-container --cluster-id CLUSTERID [--entrypoint=ENTRYPOINT] -- [CMD]so that we can run arbitrary commands in ocm-container. The original usage still currently works, but is marked Deprecated, and--cluster-idis compatible with other standard flags across other tools in use by our team.In other cases, deprecation messages are included for things that will actually be deprecated, such as the use of OCM's OFFLINE_ACCESS_TOKEN.
Feature Parity
I've tried as best as I can to have full 1:1 feature parity with the old shell script. EG: PagerDuty tokens, Personalizations, etc should work exactly the same, and be configured exactly the same. In some few cases, there may be a deprecation warning, to allow us to transition to a cleaner way of enabling these using the enhanced capabilities in golang and https://github.com/spf13/viper configuration.
All features are enabled by default, but some may require additional configuration (the same configuration required for the feature in ocm-container.sh right now). Any given feature may be disabled by using the
--no-FEATURENAME=trueCLI flag for testing, or setting theno_featurename: trueconfig or related ENV var.Note that env vars are prefixed with
OCMC_to prevent accidental parsing of system or other env vars (eg: USER).Testing and Adoption
First, unset your ocm-container alias, if you have one. The binary may be executed directly.
Get the binary: For testing this PR (not adoption or production), the
make build-snapshotcommand will use Goreleaser to build a single OS/Arch appropriate binary for you to test with.The image: ocm-container will by default use the upstream nightly build from quay.io/app-sre/ocm-container:latest. No image build is required. You may specifiy your own image. See
ocm-container --helpfor details.Initial setup:
See the README.md in this PR for a thorough documentation on how configuration and migration works.
The most basic setup can be configured via:
Optionally, existing ocm-container users can make use of a migration command to convert the env.source to a Viper ocm-container.yaml file.
Long term, I plan to expand this
ocm-container configure initcommand to prompt the user to add the values to setup ocm-container and the desired features.I would like to get as many folks involved in reviewing this and testing the functionality as possible, so we can cover all our bases and make this a seamless transition.
I also propose making a v0.1 tag for the existing master branch, and creating a pre-release v0.2 tag and binary for the contents of this PR when it merges into master, to mark the change in compatibility going forward. However, there's no concern about merging this into master, as existing folks have their clone, and the repository DOES NOT need to be cloned to get and use the Go binary, so it can be tested in parallel with the old workflow.
Comments and reviews welcome. Testing of the binary is especially! Thanks in advance!