Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable Zincati and pinger on non-release builds of production streams #212

Closed
bgilbert opened this issue Jul 6, 2019 · 7 comments
Closed

Comments

@bgilbert
Copy link
Contributor

bgilbert commented Jul 6, 2019

#163 (comment) contemplates stream-specific enablement for Zincati and the pinger. However, developers will sometimes perform test builds of production streams, e.g. in order to test backports. It's not awful if those builds try to ping or update themselves, but it's aso not helpful and adds reporting noise.

Enable Zincati and the pinger only in release builds of production streams. The code to do that should primarily live in fedora-coreos-config, since cosa shouldn't need to understand that special case in detail, and the pipeline shouldn't be mangling images behind cosa's back. @jlebon and I discussed having a manifest postprocess script look for a magic environment variable, e.g. FCOS_RELEASE_BUILD, and condition its behavior accordingly. However, rpm-ostree runs scripts in a bwrap container which doesn't pass through env vars from the cosa invocation.

@jlebon
Copy link
Member

jlebon commented Jul 6, 2019

Here's another approach which doesn't require a hidden API with the pipeline and keeps it all localized in the manifest instead:

diff --git a/fedora-coreos-base.yaml b/fedora-coreos-base.yaml
index b96e34b..8905d74 100644
--- a/fedora-coreos-base.yaml
+++ b/fedora-coreos-base.yaml
@@ -22,8 +22,9 @@ initramfs-args:
 machineid-compat: false

 releasever: "30"
-automatic-version-prefix: "30"
-mutate-os-release: "30"
+automatic-version-prefix: "${releasever}.<date:%Y%m%d>.dev"
+mutate-os-release: "${releasever}"
+
 # Be minimal
 recommends: false

@@ -122,6 +123,13 @@ postprocess:
     set -xeuo pipefail
     rm -rf /etc/systemd/system/*
     systemctl preset-all
+  - |
+    #!/usr/bin/env bash
+    set -euo pipefail
+    source /etc/os-release
+    if [[ $OSTREE_VERSION = *.dev* ]]; then
+      echo "Disabling pinger and Zincati"
+    fi

 packages:
   # Security

➡️

$ cosa build ostree
...
Executing `postprocess` inline script '5'
Disabling pinger and Zincati

(This relies on mutate-os-release, which we currently use).

The rationale for this being that we know that for release builds we'll override the OSTree version in the pipeline, so we can have the default versioning scheme hardcoded in the config include a dev marker. That way the default cosa init https://github.com/coreos/fedora-coreos-config && cosa build workflow gives you a developer image (and I think we previously discussed decorating the OSTree version for dev builds anyway).

@rfairley
Copy link

rfairley commented Jul 8, 2019

The rationale for this being that we know that in prod mode we'll override the OSTree version in the pipeline, so we can have the default versioning scheme hardcoded in the config include a dev marker.

@jlebon: By prod mode, is this a mode in the pipeline where a production image is produced from e.g. the testing-devel stream, and in that mode, the OSTree version is overridden to become the version that the production stream would use? Just want to check I'm understanding this.

I was at first thinking we could just use the stream-specific manifest.yaml to specify the postprocess script only for development branches (or alternatively the treefile field add-files for only writing snippets). That way checking for dev in the version string before disabling Zincati/pinger wouldn't be strictly necessary. This would mean though that an image built in production mode in the pipeline from the testing-devel stream would have Zincati/pinger disabled - which is probably not what we want (we'd want them enabled in that case for testing production candidate images from a developer pipeline in prod mode, I think).

Having dev (or some development marker) be in the version when building anywhere except from the fedora-coreos-pipeline in prod mode makes sense. If that condition holds, then checking for dev before disabling Zincati/pinger by default seems reasonable to me.

@bgilbert
Copy link
Contributor Author

bgilbert commented Jul 8, 2019

Can we use the term "release build" to avoid overloading the term "prod"?

@rfairley The thinking is that all branches will have Zincati and pinger disabled by default, and that will be overridden by the pipeline for official release builds, and can be overridden at build time by individual developers.

@jlebon
Copy link
Member

jlebon commented Jul 8, 2019

Can we use the term "release build" to avoid overloading the term "prod"?

Yeah, WFM. We've also been using "official" in https://github.com/coreos/fedora-coreos-pipeline for this. Anyway, updated my comment to make this clearer.

I was at first thinking we could just use the stream-specific manifest.yaml to specify the postprocess script only for development branches (or alternatively the treefile field add-files for only writing snippets)

Sorry, the terminology is super confusing here. To restate what @bgilbert said more explicitly, there are two layers here:

(1) stream-specific enablement: Zincati and pinger will have drop-ins to explicitly enable it on production streams (i.e. only on stable, testing, next). These would live in manifest.yaml on those branches.
(2) official/release build enablement: all the manifests in https://github.com/coreos/fedora-coreos-config will be optimized for developers building FCOS locally. These will yield "developer" builds, which have Zincati and pinger disabled (regardless of the actual stream/ref they built) by e.g. keying off of $OSTREE_VERSION if we use the method in #212 (comment). These could live in fedora-coreos-base.yaml so they're automatically shared across all branches.

jlebon added a commit to jlebon/fedora-coreos-config that referenced this issue Jul 10, 2019
We want to be able to distinguish between release and non-release
builds. To do this, we embed a `.dev` marker in the version string so
that the default developer workflow will use this. As mentioned in the
previous commit message, this will be overridden for release streams.

See coreos/fedora-coreos-tracker#212 for more
information about this.
jlebon added a commit to jlebon/fedora-coreos-config that referenced this issue Jul 10, 2019
On non-release/local developer builds, we don't want Zincati or pinger
to be active by default. Key off of the OSTree version and add a config
dropin to do this.

See discussions about this in:
coreos/fedora-coreos-tracker#212
@jlebon
Copy link
Member

jlebon commented Jul 10, 2019

jlebon added a commit to coreos/fedora-coreos-config that referenced this issue Jul 12, 2019
We want to be able to distinguish between release and non-release
builds. To do this, we embed a `.dev` marker in the version string so
that the default developer workflow will use this. As mentioned in the
previous commit message, this will be overridden for release streams.

See coreos/fedora-coreos-tracker#212 for more
information about this.
jlebon added a commit to coreos/fedora-coreos-config that referenced this issue Jul 12, 2019
On non-release/local developer builds, we don't want Zincati or pinger
to be active by default. Key off of the OSTree version and add a config
dropin to do this.

See discussions about this in:
coreos/fedora-coreos-tracker#212
@lucab
Copy link
Contributor

lucab commented Jul 26, 2019

@jlebon I think this is all covered and can be closed now, right?

@jlebon
Copy link
Member

jlebon commented Jul 26, 2019

Yes, agreed!

@jlebon jlebon closed this as completed Jul 26, 2019
@lucab lucab moved this from Proposed to Done in Fedora CoreOS papercuts Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

4 participants