[JENKINS-49406] Evergreen snapshotting data safety system by batmat · Pull Request #1 · batmat/jep

batmat · 2018-03-13T16:19:20Z

* LOCKED => refer now to jenkinsci#67 for the official submission**

PR to allow a first pass of reviews as I still consider this content an alpha version.

Will obviously file a PR against the final target at https://github.com/jenkinsci/jep once I feel this is ready for actual review and acceptance there.

I am well aware this might scatter feedback in two different places, but I hope to use this PR to iron out obvious issues, and officially submit upstream only once I can say there's already an agreement the proposal is decent and has been reviewed not only by myself.

Also because as per the process, I consider we are at the "Initial discussion" stage, and hence I am not supposed to submit yet.
So I'm just using a GitHub PR just for its review facilities. I could also have used Google docs or something similar for this stage of the process.

So. Feedback welcome :)

References

WIP

jtnord · 2018-03-14T14:07:25Z

jep/302/README.adoc

+
+We plan to configure Jenkins to use non default values to put the workspace and builds not under the same location as job configuration itself.
+
+_Workspace Root Directory_:: `${JENKINS_HOME}/workspace/${ITEM_FULLNAME}`


Why do you even have a workspace - executors on the master is evil and should be killed at the earliest convienience?

JENKINS-49861 FTR. I plan to have it be possible to leave an executor on the master for use by privileged jobs (JENKINS-24513). At any rate, it is worth at least reminding readers that this setting should generally have no effect.

Was a mistake @jtnord @jglick, I screwed up a copy paste when writing it.

My intent was to put both workspace and builds under the same global root, and under the same item each time.

Can you have a second look on that specific part?

jtnord · 2018-03-14T14:09:06Z

jep/302/README.adoc

+
+_Workspace Root Directory_:: `${JENKINS_HOME}/workspace/${ITEM_FULLNAME}`
+_Build Record Root Directory_::
+`${JENKINS_HOME}/var/${ITEM_FULL_NAME}/builds`


var looks a little weird. why was this chosen over something more obvious like builds
var implies you can stuff more in here - but there is not enough in the path to disambiguate from builds.

jtnord · 2018-03-14T14:10:08Z

jep/302/README.adoc

+=== Why Git
+
+Using filesystem-level tools offering a snapshotting feature, like LVM, ZFS or btrfs to give a few examples, was considered.
+But this was evacuated because _Essentials_ vision is about providing an link:https://github.com/jenkinsci/jep/tree/71d9391744c8cc7d6595805f7fdd327eedf6811a/jep/300#automatically-updated-distribution["_easier to use_ and _easier to manage_ Jenkins environment"].


evacuated? discounted?

jtnord · 2018-03-14T14:14:27Z

jep/302/README.adoc

+
+=== Man In The Middle
+
+The main issue here is that an attacker could for instance instruct the evergreen client to ignore everything (by putting `*` in `.gitignore`), hence make it impossible to rollback.


don;t use .gitignore then. use -f to add?
move builds outside JENKINS_HOME (have a directly at the mount that is /builds /whatever /config and make "config JENKINS_HOME where the usual stuff is stored).

don;t use .gitignore then. use -f to add?

Not convinced this fondamentally changes the issue. The goal is still to be able to upgrade the evergreen-client too. So, be it by hacking the .gitignore content, or by hacking what things are passed to -f I guess the issue is to really find robust ways to not accept instructions from some wrong instructors.

move builds outside JENKINS_HOME (have a directly at the mount that is /builds /whatever /config and make "config JENKINS_HOME where the usual stuff is stored).

Good point. I gave it some thoughts, but kept it under JENKINS_HOME to respect/be consistent with JEP-301:

it is expected that all mutable state and data will be written and stored in JENKINS_HOME, including: Data generated by the Jenkins process. ...

just because that is written does not mean it is correct - infact that section of JEP is bad bad bad...

why?
--pluginRoot --webroot. JENKINS_HOME is possibly mounted on a resilient storage (and is usually slower or more costly) there is no need to unpack the Jenkins plugins or war into jenkins-home...

Also logs... Hmmm if they are in jenkins home and you rollback then you loose some logs..
that;s not a good thing is it?

and well if you take it at face value...

"No other data should be written to the filesystem outside of JENKINS_HOME."

build artifacts are data... yet with evergreen you want to move these to S3 or somewhere not the filesystem. that JEP would preclude that work. //cc @rtyler

now if jep-301 wanted to say thal all data be contained under a single directory then fine (but JENKINS_HOME does not need to be at the root of that directory) IMO and if you are making a more complex system because of that descision then go revisit it...

jglick · 2018-03-14T15:14:40Z

jep/302/README.adoc

@@ -0,0 +1,260 @@
+= JEP-302: Evergreen snapshotting data safety system


You do not pick the number when submitting a JEP. See the instructions.

Fixed thanks. Didn't rename the folder though to avoid GitHub folding all comments below despite me not having addressed them yet. Will do when submitting upstream.

I think GitHub is smart enough to handle rename detection, but I can understand not wanting to test that now!

jglick · 2018-03-14T15:20:03Z

jep/302/README.adoc

+7. [[healthcheck]] Check it is responding in a healthy manner.
+8. If not:
+.. Take a snapshot footnote:[this way, if new files were created, we don't just delete them in an unrecoverable way when going back to the previous snapshot].
+.. Rollback to the previous data snapshot and Essentials BOM version.


Roll back

jglick · 2018-03-14T15:20:40Z

jep/302/README.adoc

+8. If not:
+.. Take a snapshot footnote:[this way, if new files were created, we don't just delete them in an unrecoverable way when going back to the previous snapshot].
+.. Rollback to the previous data snapshot and Essentials BOM version.
+To do so, we will generally create a new commit (i.e. avoid `git reset --hard HEAD~`), to keep a durable track of where we went through, accessible through `git log`.


So, git revert?

Yes, basically.

UPDATE: fixed

jglick · 2018-03-14T15:21:09Z

jep/302/README.adoc

+To do so, we will generally create a new commit (i.e. avoid `git reset --hard HEAD~`), to keep a durable track of where we went through, accessible through `git log`.
+.. Start (previous) Jenkins version
+// what if starting the previous version doesn't work either?
+9. Report the outcome to the Essentials backend.


i.e., let the heads start rolling

Well, I hope this is clear, but here I meant that we want to report the outcome, whatever it is, to the backend. We also very much need the result if everything goes well, to be able to push it to more instances.

jglick · 2018-03-14T15:21:29Z

jep/302/README.adoc

+
+* Update `.gitignore` content with current Essentials release.
+* `git add .`
+* `git add -u `


stray space breaks formatting

jglick · 2018-03-14T16:07:50Z

jep/302/README.adoc

+=== Secrets
+
+Versioning secrets is not an issue, as the snapshotting system is going to be local to the running instance.
+As the Git repository data will never be pushed _outside_, no data leak is expected from this side.


I do not think this is acceptable. Someone will use the Git repository externally regardless of what you advertise.

Fortunately there is a simple solution: add

secrets/master.key

to .gitignore. It is created on first startup and never changed thereafter, so it would never be affected by an upgrade or downgrade. And without it, all other secrets remain (theoretically) secret.

Reference

Well, I do not feel strongly to add it or ignore it. But well, if someone is able to access the FS and so what you are saying, in any case they have the power to read that key.

I understand what you are saying as "someone could have the bad idea to push that repo elsewhere, when it's not supposed to be, not groking the fact they're going to leak central secrets doing so, and so we'll be seen as responsible for it.".

So yes, let's be conservative and not encourage people to shoot themselves in the foot.

FTR, fixed.

jglick · 2018-03-14T16:09:28Z

jep/302/README.adoc

+
+* Apply the upgrade or downgrade, then check the instance is _running fine_:
+** No administrative monitor triggered
+** No new ERROR logs showing up


You mean, Level.ERROR? Hardly useful: few plugins log at this level to begin with. Almost everything of interest would be a WARNING.

Yes and no. But your point makes me realize I really need to externalize this concept. We need a standalone design for determining Jenkins health I think. It's a critical condition/piece for many components: for testing, for evergreen-client to decide to rollback or not, etc. And it is not (necessarily) related to the data snapshotting system.

So will refactor to make this generic and point to something to be done in this regard.

Also commented in https://issues.jenkins-ci.org/browse/JENKINS-49406?focusedCommentId=332233&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-332233 about that

jglick · 2018-03-14T16:09:44Z

jep/302/README.adoc

+** No administrative monitor triggered
+** No new ERROR logs showing up
+
+We will need to develop adhoc testing tools to be able to automatically assess the health of a Jenkins Essentials instance after an upgrade or a downgrade.


jglick · 2018-03-14T16:13:48Z

jep/302/README.adoc

+
+Before delivering updates on real connected instances, we must test at least the following scenarios.
+
+* Apply the upgrade or downgrade, then check the instance is _running fine_:


I think you are missing something critical here: you need to be able to ensure that nothing interesting happening in the system between the time Jenkins completes initialization after the upgrade, and the time the health check completes. If builds run, that is probably fine (as I argue above w.r.t. build record location), though a QueueTaskDispatcher or just setQuietDown could block builds temporarily after startup. But what about users editing job or system configuration? You would need to put some kind of UI block in place, I think.

The broader question which I guess you are punting on is how useful a simplistic health check like this really is. Just starting without warnings is pretty useless if every build you try to run bombs out with some fatal low-level error. But then if you let some trial builds run, you are widening the window after which a revert could be problematic.

jglick · 2018-03-14T16:14:38Z

jep/302/README.adoc

+Detailing this here is out of scope for this proposal.
+It is however highly desirable that we centralize this logic and use it both during automated tests, and in production for the evergreen-client to automatically analyze if a product instance is healthy or is not (and decide to rollback or not, for the current matter here).
+
+We will leverage the link:https://github.com/cloudbees/operations-center-acceptance-test/[Jenkins Acceptance Test Harness project] for this purpose.


I think you meant to link to something else.

oooops fixed

I was not supposed to choose the number myself, obviously... Thanks Jesse for the reminder. Not renaming the folder yet, on purpose, to avoid folding all other review comments although I didn't address them yet.

Jesse is basically saying that despite the system/git repo is not designed to be pushed outside, (some) people will do so...

Was trying to use the title of a code block, starting with a dot. But here asciidoctor was confused by the .. at ..gitignore. So using the clearer form title=blah to make it work (and be less magic in the same go, which is good).

The intent here was to have a single root for builds and workspace for a given item.

batmat · 2018-03-15T16:27:52Z

Reminder for me, and for anyone interested, things I want to address:

About the point above by @jglick and @jtnord on moving $JENKINS_HOME/var out of $JENKINS_HOME, I opened a dedicated thread for feedback on the dev list. I am leaning towards I'm going to adjust the proposal with this. It makes sense to me.
Another important point above by @jglick which I'm summarizing as "how do we check Jenkins is actually healthy before opening the flood gates to users: letting jobs execute, people modify things through the UI, etc.". This point is probably the most critical and needs addressing.

If there is anything else important I missed, please don't hesitate to tell me so.

…arately

Proposal to address Jesse's comment: > The broader question which I guess you are punting > on is how useful a simplistic health check like this > really is. Just starting without warnings is pretty > useless if every build you try to run bombs out with > some fatal low-level error. But then if you let some > trial builds run, you are widening the window after > which a revert could be problematic."

as it should have been since the beginning

jglick · 2018-03-21T13:02:00Z

jep/302/README.adoc

+The intent is to have everything not ignored in the index.
+In other words, `git status` just after a SNAPSHOT should be in the `nothing to commit, working tree clean` state.
+
+We will *not* snapshot data that is too big or does not make sense to store:.


jglick · 2018-03-21T13:18:59Z

jep/302/README.adoc

+│   ├── nodes
+│   ├── plugins
+│   ├── secrets
+│   ├── updates


can be ignored I think

Added to .gitignore.

jglick · 2018-03-21T13:20:21Z

jep/302/README.adoc

+
+This way, everything related to jobs, but is less important than its configuration itself is separated under a different `${JENKINS_HOME}/var/` root.
+
+Though we will for now back up these directories too, having this distinction baked in from the ground up could prove useful in the future if we need to make the snapshotting and rollback processes smarter and more robust.


Wait, I thought we were only backing up /var/jenkins/home?

yes, that's it. Reading, maybe I forgot to adjust a sentence.

Fixed, removed that part.

jglick · 2018-03-21T13:25:06Z

jep/302/README.adoc

+====== about `/var/jenkins/home/plugins`
+
+This directory contains the hpi/jpi files before extraction.
+Ideally, we should be moving this elsewhere under `/var/jenkins/var/plugins`, but it's not doable yet currently (`--pluginsroot` only configures a different location for exploded plugins).


Careful here. Currently $JENKINS_HOME/plugins/shortName.jpi is the definitive record that a plugin was installed (there is also a shortName.jpi.disabled file if disabled), and in what version. If you do not back that up, then you are no longer versioning the Jenkins configuration.

Now I understand that for plugins in Essentials, this is exactly right—the Essentials manifest is in control of what plugins are there. (Again, excepting *.jpi.disabled files, so I think what you meant to exclude is more narrowly plugins/*.jpi.) But what about “inessential” plugins? There will be no trace in the Git history of their presence or version.

Admittedly a rollback is not going to need to revert these, either, since the Essentials upgrade would not have touched them. Just seems a little risky. People may be expecting that a Git revision (plus master.key) corresponds to a fully reconstructible Jenkins instance.

Maybe the .gitignore can simply list those plugins actually in Essentials?

plugins/structs.jpi # …

People may be expecting that a Git revision (plus master.key) corresponds to a fully reconstructible Jenkins instance.

Well, then they will be deceived... It's not a backup system as we already discussed.

To me that this part is going to be out of the scope of the snapshotting system, exactly for the reason you gave: I consider that the whole plugin list is defined explicitly by the BOM, for Essentials plugin.

I would say that for others, well it's also out of scope, and it will be the evergreen client responsibility to store the versions of all plugins present there, to be able to restore the previous version if we rollback.

jglick · 2018-03-21T13:25:15Z

jep/302/README.adoc

+=== Checking Jenkins health
+
+From the perspective of this proposal, this is out of scope.
+But the outter _controller_ of the upgrade, the evergreen client, will need a way to decide if a rollback must be triggered or not.


jglick · 2018-03-21T13:25:53Z

jep/302/README.adoc

+If a version is proved to be problematic after a few days, the data snapshotting system will **not** be used.
+
+This would be quite impractical because the instance probably generated actual work items during this timeframe.
+So rolling back that much later would be subject to losing data.


grammar: would risk data loss

also: rolling that back (separable verbs, sigh)

done thanks

jglick · 2018-03-21T13:28:19Z

jep/302/README.adoc

+Versioning secrets should not be an issue per se, as the data snapshotting system is designed to be local to the running instance.
+In other words, the Git repository data will never be pushed _outside_ by the _Essentials_ code, so no data leak is normally expected from this side.
+
+But as users may have the unfortunate idea to push that repository elsewhere, not being aware they are leaking all secrets, we will conservatively add `secrets/master.key` to the `.gitignore` file.


Would be a good place to briefly explain what this file is and why excluding it from the Git repository prevents any straightforward extraction of secrets.

I would like to avoid (badly) repeating something that probably ought to be defined in the official docs. But can add a link.

Something like I guess:

"Secrets are encrypted using the master.key files, so we really want to avoid attackers to be able to access both credentials.xml and master.key at the same time

?

batmat · 2018-03-21T14:36:33Z

Now submitted for official review at jenkinsci#67

batmat force-pushed the JENKINS-49406 branch from b51a78f to 62eb984 Compare March 13, 2018 16:22

batmat changed the title ~~[JENKINS-49406] WIP Evergreen snapshotting data safety system~~ WIP [JENKINS-49406] Evergreen snapshotting data safety system Mar 14, 2018

batmat force-pushed the JENKINS-49406 branch from 62eb984 to c7fdacb Compare March 14, 2018 11:51

[JENKINS-49406] WIP Evergreen snapshotting data safety system

233ea29

WIP

batmat force-pushed the JENKINS-49406 branch from c7fdacb to 233ea29 Compare March 14, 2018 12:03

batmat changed the title ~~WIP [JENKINS-49406] Evergreen snapshotting data safety system~~ [JENKINS-49406] Evergreen snapshotting data safety system Mar 14, 2018

Precision why compatibleSinceVersion is not used (for now at least)

a319d4b

jtnord reviewed Mar 14, 2018

View reviewed changes

jglick reviewed Mar 14, 2018

View reviewed changes

Back to JEP-0000

8a626b3

I was not supposed to choose the number myself, obviously... Thanks Jesse for the reminder. Not renaming the folder yet, on purpose, to avoid folding all other review comments although I didn't address them yet.

batmat force-pushed the JENKINS-49406 branch from 3c75504 to 8a626b3 Compare March 15, 2018 10:10

batmat added 15 commits March 15, 2018 11:18

rollback must be roll back when used as a verb

38e9957

Add secrets/master.key to .gitignore

b271914

Jesse is basically saying that despite the system/git repo is not designed to be pushed outside, (some) people will do so...

Woops, countersense

5ee0803

Small formatting issues

0f3ec5d

Clarify that revert is meant

ab866c5

Use git add --all instead of a convoluted git add . + git add -u

4c5c5b0

back up, not backup, when used as a verb

3fc91c5

Formatting issue

446a4e5

Was trying to use the title of a code block, starting with a dot. But here asciidoctor was confused by the .. at ..gitignore. So using the clearer form title=blah to make it work (and be less magic in the same go, which is good).

Sigh, critical copy-paste error...

de6e855

The intent here was to have a single root for builds and workspace for a given item.

evacuated => discounted

c0198a7

Clarify a bit more the role of ${JENKINS_HOME}/var/

6f65b47

adhoc => ad-hoc

5573eb3

spurious char

c871dff

typo

179521a

Wrong link...

03d3404

batmat added 3 commits March 16, 2018 15:25

Data snapshotting system is NOT a backup system

a3d7091

Fix wrong title level

e7bbb8c

Adjust the data segregation part of the proposal

9d910f1

Clarify that determining the health of an instance is to be done, sep…

fc65aa4

…arately

batmat mentioned this pull request Mar 20, 2018

[JENKINS-49406] Start segregating data a bit more jenkins-infra/evergreen#27

Merged

batmat force-pushed the JENKINS-49406 branch from 6897584 to 19e3e76 Compare March 20, 2018 22:16

batmat force-pushed the JENKINS-49406 branch from 19e3e76 to e3320b6 Compare March 20, 2018 22:17

Heavily rephrase the compatibleSinceVersion section

53be8b3

batmat force-pushed the JENKINS-49406 branch from 3f32d19 to 53be8b3 Compare March 21, 2018 11:02

Final adjustments, leaving the health checking out of scope

7855617

as it should have been since the beginning

jglick reviewed Mar 21, 2018

View reviewed changes

batmat added 3 commits March 21, 2018 14:39

Remove leftover from previous versions

27de4ae

outter => outer

906fb38

better grammar, thanks Jesse

e176ef8

batmat mentioned this pull request Mar 21, 2018

[JENKINS-49406] Evergreen snapshotting data safety system JEP jenkinsci/jep#67

Merged

batmat closed this Mar 21, 2018

batmat deleted the JENKINS-49406 branch March 21, 2018 14:36

Repository owner locked as resolved and limited conversation to collaborators Mar 21, 2018


		We plan to configure Jenkins to use non default values to put the workspace and builds not under the same location as job configuration itself.

		_Workspace Root Directory_:: `${JENKINS_HOME}/workspace/${ITEM_FULLNAME}`


		=== Man In The Middle

		The main issue here is that an attacker could for instance instruct the evergreen client to ignore everything (by putting `*` in `.gitignore`), hence make it impossible to rollback.

		@@ -0,0 +1,260 @@
		= JEP-302: Evergreen snapshotting data safety system


		Before delivering updates on real connected instances, we must test at least the following scenarios.

		* Apply the upgrade or downgrade, then check the instance is _running fine_:


		This way, everything related to jobs, but is less important than its configuration itself is separated under a different `${JENKINS_HOME}/var/` root.

		Though we will for now back up these directories too, having this distinction baked in from the ground up could prove useful in the future if we need to make the snapshotting and rollback processes smarter and more robust.

Conversation

batmat commented Mar 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

* LOCKED => refer now to jenkinsci#67 for the official submission**

References

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

batmat Mar 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtnord Mar 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

batmat Mar 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

batmat commented Mar 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

batmat commented Mar 13, 2018 •

edited

Loading

batmat Mar 15, 2018 •

edited

Loading

jtnord Mar 15, 2018 •

edited

Loading

batmat Mar 14, 2018 •

edited

Loading

batmat Mar 21, 2018 •

edited

Loading