Tails

#3 Updated by bertagaz 2017-06-09 10:39:35

intrigeri wrote:
> I’ll take care of it unless you want it.

As you wish. That’s easy and fun to set up and good to play with our Jenkins jobs setup, but if you ever want to do something else, feel free to reassign to me.

#4 Updated by intrigeri 2017-06-09 14:08:19

> As you wish. That’s easy and fun to set up and good to play with our Jenkins jobs setup, but if you ever want to do something else, feel free to reassign to me.

Sure!

#10 Updated by bertagaz 2017-10-18 14:04:04

intrigeri wrote:
> Please go ahead.

OK, so I’ve spent time trying to get how to implement that. That’s sadly not so easy.

The usual way to do it would be to trigger a build of the reproducibly_build_Tails_ISO job from the build_Tails_ISO job. But if we do that, we’d try to reproduce every ISOs that are built in Jenkins, that’s not what we want to do.

Other ways to trigger a build are not really usable: one of them is a builder (in JB language), which means it would be run before the build_Tails_ISO job archives its ISO. Another way is to use a build-result trigger in the reproducibly_build_Tails_ISO job, that would watch if a related build_Tails_ISO job has succeeded. But that sounds like a racy solution, as it uses a cronjob to periodically check if a upstream job succeeded. What happen if another one did in the meantime it fires up a reproducibly_build_Tails_ISO job, it’s quite unclear, as it does not seem to have ways to refer to a particular upstream build number.

So the only way I’ve found to do that is to use the conditional-step plugin in the reproducibly_build_Tails_ISO job. It uses a script to determine whether it’s building a base branch and thus should run the whole reproducibly_build_Tails_ISO job. Bonus point is that with that we can also check if the branch ticket is RfQA (cf Feature #13436).

Downside of this solution is that this plugin depends on other plugins version that are a bit newer than what we have. So that’s very hard to say if it would work in our current infra.

The main blocker is the token-macro plugin that requires to be 1.51 in any version of the conditional-step plugin. But reading the later plugin source, it may be that it works well with our current version of the token macro plugin. It may also be that upgrading the token macro plugin won’t break anything, but a lot of other plugins depend on it. So that’s hard to say, and I don’t know how to cope with that appart from tackling Bug #10068, which is waaaaay out of scope.

I’ve pushed my work in the jenkins-jobs:feature/12633-lower-reproducible-builds-workload branch and puppet-tails:feature/12633-lower-reproducible-builds-workload if you want to have a look.

#14 Updated by intrigeri 2017-10-20 09:06:17

> The usual way to do it would be to trigger a build of the reproducibly_build_Tails_ISO job from the build_Tails_ISO job. But if we do that, we’d try to reproduce every ISOs that are built in Jenkins, that’s not what we want to do.

Indeed. To clarify, if we were only caring about this ticket i.e. base branches, it would be good enough: we could simply enable this trigger in the build jobs we generate for base branches. But Feature #13436 is also part of the plan, so that won’t be good enough.

> Another way is to use a build-result trigger in the reproducibly_build_Tails_ISO job, that would watch if a related build_Tails_ISO job has succeeded. But that sounds like a racy solution,

Thanks for staying away from solutions that require reasoning about asynchronous operations. This seems wise to me.

#17 Updated by bertagaz 2017-10-20 10:04:01

intrigeri wrote:
> My biggest worry is: the ISO test job be started in parallel with the reproducibly_build_Tails_ISO_{name} one, or will it block on that one to be completed first?

From what I understood, the jobs will be parallelized, as they’re not running on the same slave, and the test job does not depend on the reproducible job.

> It’s fine to duplicate the ticket_qa_check code we already had during the PoC phase. But please refactor before this is called done (we already have a Python library so it should be quite easy).

Opened a ticket referenced here for that, because it’s a bit more work and time to get that in place in the infra. That’s less urgent than this ticket.

> What’s the purpose of checkout_upstream_base_branch_head?

Well, we need to checkout the exact same state of the Git repo compared to the upstream build job, including the base branch at the same HEAD. Between the git clone stage in the build job, and the one in the reproducible job, there may have been pushes in the base branch.

> I’m not a big fan of the list of maybe_* builders in reproducibly_build_Tails_ISO.yaml; the flow control is unclear to me (e.g. will move_artifacts_to_dir_2 run if the 2nd ISO build fails?), it feels wrong to check the same boolean condition N times, and quite simply it’s very unclear what “maybe” means (one can’t know before reading the implementation, which raises a red flag with “badly named function” written on it). How about checking and aborting early if the boolean flag file is not present?

How do you do that in Jenkins? A build step is supposed to run successfuly or fail the build, it does not control the rest of the build steps chain and flow.

> Linking to the artifacts of the last successful reproducibly_build_Tails_ISO_* job seems to result of a mechanical change without much thought put into the resulting developer experience (which might be OK-ish at PoC stage but let’s please adopt a user-centric PoV ASAP in the design & development process, because experience showed that it’s hard to patch this back in later on): when an ISO can be reproducibly built a 2nd time, who cares about the artifacts? As a developer I’m more interested in 1. the artifacts for the last reproduction failure (that link could be adjusted accordingly, I guess); 2. whether the last reproduction attempt was successful (IIRC I can see that in the project’s sidebar so I guess it doesn’t need another link).

I’m not sure to get whay you mean here. What link where, and what ‘mechanical change’?

#18 Updated by intrigeri 2017-10-20 10:38:58

>> It’s fine to duplicate the ticket_qa_check code we already had during the PoC phase. But please refactor before this is called done (we already have a Python library so it should be quite easy).

> Opened a ticket referenced here for that, because it’s a bit more work and time to get that in place in the infra. That’s less urgent than this ticket.

Deal. I’m fine with postponing this a little bit but I’ve adjusted metadata of Bug #14871 to clarify what chunk of work it’s part of.

>> What’s the purpose of checkout_upstream_base_branch_head?

> Well, we need to checkout the exact same state of the Git repo compared to the upstream build job, including the base branch at the same HEAD. Between the git clone stage in the build job, and the one in the reproducible job, there may have been pushes in the base branch.

I understand we need to use the base branch in the state it was in during the initial build. So far, so good. But I don’t understand how the checkout part will help: in auto/config we do git merge --no-edit "origin/${GIT_BASE_BRANCH}", so I don’t see what checking out a local branch has to do with it. Perhaps I’m confused because I don’t know what origin points to in the context of this job. Anyway, I don’t care much and I’m inclined to trust you on this one so let’s not bother. Please just ensure it works as intended when the whole thing is tested :)

>> I’m not a big fan of the list of maybe_* builders in reproducibly_build_Tails_ISO.yaml; the flow control is unclear to me (e.g. will move_artifacts_to_dir_2 run if the 2nd ISO build fails?), it feels wrong to check the same boolean condition N times, and quite simply it’s very unclear what “maybe” means (one can’t know before reading the implementation, which raises a red flag with “badly named function” written on it). How about checking and aborting early if the boolean flag file is not present?

> How do you do that in Jenkins?

I don’t know. Sorry, I can’t help you design this given the time constraints you’re working under. The best I can do is reviewing.

> A build step is supposed to run successfuly or fail the build, it does not control the rest of the build steps chain and flow.

Then perhaps a chain of super-fine-grained build steps is not the right approach?

>> Linking to the artifacts of the last successful reproducibly_build_Tails_ISO_* job seems to result of a mechanical change without much thought put into the resulting developer experience (which might be OK-ish at PoC stage but let’s please adopt a user-centric PoV ASAP in the design & development process, because experience showed that it’s hard to patch this back in later on): when an ISO can be reproducibly built a 2nd time, who cares about the artifacts? As a developer I’m more interested in 1. the artifacts for the last reproduction failure (that link could be adjusted accordingly, I guess); 2. whether the last reproduction attempt was successful (IIRC I can see that in the project’s sidebar so I guess it doesn’t need another link).

> I’m not sure to get whay you mean here. What link where, and what ‘mechanical change’?

I’m talking about the links you’ve modified yesterday (in the non-atomical commit 11497165e109185df9a7e915f4028967b44643f8 in jenkins-jobs.git) as part of the branch you asked me to look at.

#19 Updated by bertagaz 2017-10-20 13:27:42

intrigeri wrote:
> >> What’s the purpose of checkout_upstream_base_branch_head?
>
> I understand we need to use the base branch in the state it was in during the initial build. So far, so good. But I don’t understand how the checkout part will help: in auto/config we do git merge --no-edit "origin/${GIT_BASE_BRANCH}", so I don’t see what checking out a local branch has to do with it. Perhaps I’m confused because I don’t know what origin points to in the context of this job. Anyway, I don’t care much and I’m inclined to trust you on this one so let’s not bother. Please just ensure it works as intended when the whole thing is tested :)

In this step origin of this repo points to the immerda repo. In the cloned repo of this one, where the merge and build happen (so in Vagrant) in the build step, the origin is artificially set to the same immerda URI.

So if the HEAD of the base branch on immerda has changed between the cloning in the build_Tails_ISO job and the one in the reproducibly_build_Tails_ISO, we won’t merge the same base branch state.

We need to fake that the origin (immerda) base branch state is at the HEAD we want, so that in the Vagrant repo clone we’ll merge the same fake immerda origin base branch state.

From my tests, git-update-ref is the good tool for that.

> >> I’m not a big fan of the list of maybe_* builders in reproducibly_build_Tails_ISO.yaml; the flow control is unclear to me (e.g. will move_artifacts_to_dir_2 run if the 2nd ISO build fails?), it feels wrong to check the same boolean condition N times, and quite simply it’s very unclear what “maybe” means (one can’t know before reading the implementation, which raises a red flag with “badly named function” written on it). How about checking and aborting early if the boolean flag file is not present?
>
> > How do you do that in Jenkins?
>
> I don’t know. Sorry, I can’t help you design this given the time constraints you’re working under. The best I can do is reviewing.

I asked because I don’t know any means to do that in Jenkins myself, hence my design choices.

> > A build step is supposed to run successfuly or fail the build, it does not control the rest of the build steps chain and flow.
>
> Then perhaps a chain of super-fine-grained build steps is not the right approach?

If you’re implying we have to make a wrapper to run this 3 “maybe” steps, then I’m not sure it makes sense. That wrapper would run the build, download the ISO and compare both ISOs. I think having tiny atomic steps for this different things is much more clear, including when one of this step fails. In the case of a wrapper, we’ll have to dig a deep further in the logs to get why the job failed. Maybe we have to find a better naming scheme rather?

#20 Updated by bertagaz 2017-10-20 15:37:15

intrigeri wrote:
> The File['/usr/local/bin/decide_if_reproduce'] resource should depend on the script’s dependencies (e.g. the Python Redmine library).

commit puppet-tails:657d1cb

> Do we still need if args.apikey_file is None now that that argument is required?
>
> Now that arguments are required and the main logic is about computing a boolean value, the code could express what it does in a much more straightforward way with something like (untested):

Right, commit puppet-tails:f58466e

> “the reproducible rebuild” feels wrong: we don’t know yet if it’ll be reproducible, the whole purpose is to check whether it is. Maybe “build reproducibility test” instead? In a few other places “reproduce” is used in ways that have the same semantics problem.

commit puppet-tails:a844c68

> > Linking to the artifacts of the last successful reproducibly_build_Tails_ISO_* job seems to result of a mechanical change without much thought put into the resulting developer experience (which might be OK-ish at PoC stage but let’s please adopt a user-centric PoV ASAP in the design & development process, because experience showed that it’s hard to patch this back in later on): when an ISO can be reproducibly built a 2nd time, who cares about the artifacts? As a developer I’m more interested in 1. the artifacts for the last reproduction failure (that link could be adjusted accordingly, I guess); 2. whether the last reproduction attempt was successful (IIRC I can see that in the project’s sidebar so I guess it doesn’t need another link).
>
> I’m talking about the links you’ve modified yesterday (in the non-atomical commit 11497165e109185df9a7e915f4028967b44643f8 in jenkins-jobs.git) as part of the branch you asked me to look at.

Ah, ok, commit jenkins-jobs:ae00672.

I’ve also simplified and improved a bit more the steps with commits jenkins-jobs:2df9768 and jenkins-jobs:11b0f1cf

#21 Updated by intrigeri 2017-10-21 07:16:08

Hi!

> intrigeri wrote:
>> >> What’s the purpose of checkout_upstream_base_branch_head?
>>
>> I understand we need to use the base branch in the state it was in during the initial build. So far, so good. But I don’t understand how the checkout part will help: in auto/config we do git merge --no-edit "origin/${GIT_BASE_BRANCH}", so I don’t see what checking out a local branch has to do with it. Perhaps I’m confused because I don’t know what origin points to in the context of this job. Anyway, I don’t care much and I’m inclined to trust you on this one so let’s not bother. Please just ensure it works as intended when the whole thing is tested :)

> In this step origin of this repo points to the immerda repo. In the cloned repo of this one, where the merge and build happen (so in Vagrant) in the build step, the origin is artificially set to the same immerda URI.

> So if the HEAD of the base branch on immerda has changed between the cloning in the build_Tails_ISO job and the one in the reproducibly_build_Tails_ISO, we won’t merge the same base branch state.

> We need to fake that the origin (immerda) base branch state is at the HEAD we want, so that in the Vagrant repo clone we’ll merge the same fake immerda origin base branch state.

Thanks a lot for the explanation, it makes sense to me.

> From my tests, git-update-ref is the good tool for that.

OK. The branch was not using git update-ref last time you asked me to look at it. Now I see it does, and indeed it makes much more sense to me this way.

>> >> I’m not a big fan of the list of maybe_* builders in reproducibly_build_Tails_ISO.yaml; the flow control is unclear to me (e.g. will move_artifacts_to_dir_2 run if the 2nd ISO build fails?), it feels wrong to check the same boolean condition N times, and quite simply it’s very unclear what “maybe” means (one can’t know before reading the implementation, which raises a red flag with “badly named function” written on it). How about checking and aborting early if the boolean flag file is not present?
>>
>> > How do you do that in Jenkins?
>>
>> I don’t know. Sorry, I can’t help you design this given the time constraints you’re working under. The best I can do is reviewing.

> I asked because I don’t know any means to do that in Jenkins myself, hence my design choices.

OK. Apparently you did have an implementation (with conditional-step) that expressed more clearly what the deal is: before commit 11497165e109185df9a7e915f4028967b44643f8 (jenkins-jobs) that introduced the maybe_ confusion, things looked just fine. So from my non-expert PoV, it does seem to be possible to express cleanly what you mean in Jenkins.

>> > A build step is supposed to run successfuly or fail the build, it does not control the rest of the build steps chain and flow.
>>
>> Then perhaps a chain of super-fine-grained build steps is not the right approach?

> If you’re implying we have to make a wrapper to run this 3 “maybe” steps, then I’m not sure it makes sense. That wrapper would run the build, download the ISO and compare both ISOs. I think having tiny atomic steps for this different things is much more clear, including when one of this step fails. In the case of a wrapper, we’ll have to dig a deep further in the logs to get why the job failed. Maybe we have to find a better naming scheme rather?

I don’t understand but it’s not important (I don’t want to dive into this topic too deep myself). Don’t bother explaining the detailed reasons to me, I’ll focus on your actual code and will refrain wasting your time with random, poorly thought suggestions :)

I’ll look at this again tomorrow morning if this ticket has been assigned to me in the meantime (if you do that, please make it clear if/how the whole thing has been tested, and bonus points if you give me hints wrt. testing it myself on my local Jenkins setup). Then as you know, starting on Monday I’ll be away from Tails for a while.

#27 Updated by intrigeri 2017-10-29 08:26:10

The contract ends in 2 days. Do you still plan to deploy (and fix as needed) stuff by the end of the month? If not, please do communicate about your plans.

#28 Updated by bertagaz 2017-10-30 10:03:45

intrigeri wrote:
> The contract ends in 2 days. Do you still plan to deploy (and fix as needed) stuff by the end of the month? If not, please do communicate about your plans.

Yes.

#29 Updated by intrigeri 2017-10-30 16:58:39

>> The contract ends in 2 days. Do you still plan to deploy (and fix as needed) stuff by the end of the month? If not, please do communicate about your plans.

> Yes.

Amazing! :)

#32 Updated by intrigeri 2017-11-06 20:13:03

bertagaz wrote:
> It’s been deployed and the few remaining bugs and adjustements have been fixed in commits referencing this ticket, in the puppet-tails and jenkins-jobs repos.

> Working scenarios can be seen in today’s builds and reproducible builds of the stable and Bug #10494 branch. The latest having being tested depending on its ticket Qa Check status. The same can be done to test how the jobs behave, on this branch (I don’t care a few email), or on the reviewer’s own branch.

I had a first quick look and the first thing I’ve noticed is Bug #14924: it’s definitely not your job to fix it, but please pay some more attention to what your work gives us (or fails to) in practice, and when you notice it actually gives us no useful info at all for one of our most important branches, please report it so whoever else feels responsible for it can fix the root cause. Thanks in advance!

I’ll fix Bug #14924 before I look at this closer; I might have to postpone this review to the 3.4 cycle given I had to take over quite some work from one of our colleagues; anyway, if I’m not mistaken there’s no hurry anymore as the deadline has passed and this was deployed in production pre-review already.

> I’ll write and email to tails-dev asap.

Yes, please let your users know about this change. You know the drill, not teaching them to ignore the very notifications we want them to read and all that.

#33 Updated by intrigeri 2017-11-08 11:08:08

First issues I’ve noticed with my “user of this system in real life” hat (for now I don’t care about the implementation details, I only care about my UX):

• I see the tails-reproduce file is archives as an artifact. This is confusing and distracting because I wonder “is there any info I should care about in there”? Granted, it’s a minor issue in isolation, but these things add up.
• The reproducibly_* jobs are marked as succeeded when no actual rebuild was done. So if I’m reviewing a branch, in order to know whether it breaks reproducibility, I have to look at two things: the corresponding reproducibly_* job status and the duration of the last run. If I only look at the status, then I might merge a branch whose reproducibility has not been checked yet, e.g. because I am reviewing it before our CI system noticed it’s now Ready for QA and flagged it for reproducibility testing accordingly (which would then be checked in the next 24h if I’m not mistaken). This is not consistent with how we display test suite run results and does not match my habits. I don’t think it’s OK to expect a reviewer to check the results of these two kinds of test jobs in two different ways. What I would like to see instead is: ideally, some kind of “not built” or “skipped” status (like the grey icon when a job has never run yet); worst case, if you can’t implement proper semantics, I think I prefer to see a reproducibility job that actually didn’t rebuild as failed rather than as successful. But wait, why do we generate these reproducibly_build* jobs at all when we already know they’ll be no-op and just add noise to the UI? It looks like we’re in some grey area between two designs that would make sense to me: either only create jobs that will actually do something useful, or generate them all and check at run time whether a rebuild must be done (== if Ready for QA, or base branch, or something). I’ll let you figure out how to fix this UX problem. It’s OK if the short-term fix is not super nice, as long as there’s some kind of plan to make it nicer later on (I’d rather see this fixed ASAP rather than blocking on whatever big thing is needed to make the whole thing more consistent).

#34 Updated by intrigeri 2017-11-08 11:41:10

Also, I suggest renaming the jobs: the reproducibly_build_ prefix does not correctly convey what they’re doing: we don’t know if they’re going to build “reproducibly”, that’s precisely what they’re trying to test. What about test_reproducibility_ instead?

#35 Updated by intrigeri 2017-11-08 16:59:41

bertagaz wrote:
> I get it’s not easy in this case, so I reverted this artifact archiving step to archive both ISOs.

Thanks, I’ve taken a look and indeed that’s much better :)

> Now we planned this duplicate storage for our base branches only, not when we also try to reproduce branches for which ticket is RfQA.

What do you plan to do about it? (I see no corresponding ticket related to this one.)

#38 Updated by intrigeri 2017-11-10 15:36:36

intrigeri wrote:
> I think there’s a bug in the handling of the base branch merge or something related: https://jenkins.tails.boum.org/view/Tails_ISO/job/reproducibly_build_Tails_ISO_bugfix-14796-topicons-position/8/artifact/build-artifacts/diffoscope.html shows differing (merge) commit IDs, while the build logs pretend we’re merging the exact same commit (“at commit …”). Neither of these two commit IDs are in the official tails.git, hence my guess that a different merge commit ID is created for some reason unknown to me. I don’t know what’s going on here but it seems fishy. Please investigate and if this is Someone Else’s Problem™ file a ticket for them, otherwise fix it. I suspect it would help debugging if we passed --verbose to this git merge, and/or displayed the ID of the resulting merge commit, or similar.

I was annoyed by this bug while working on Bug #14933 so I’ve investigated a bit and filed Bug #14946. I’ll take care of it.

#42 Updated by intrigeri 2017-12-17 15:02:27

When I receive email notifications like “Build failed in Jenkins: reproducibly_build_Tails_ISO_bugfix-15029-apparmor-cups-backends”, I would like to be able to quickly dismiss the obvious cause of non-reproducibility (different APT snapshots used for the 2nd build). Currently I don’t do it because it simply happens too often, there’s no direct link to the diffoscope output, and I can’t be bothered switching to my web browser to go to the Jenkins web UI. But I’m pretty sure I would bother investigating if the email notification directly included the content of diffoscope.txt. So I propose that after running diffoscope, if diffoscope.txt is small enough (say, not more than 64KB), we cat it so its content ends up in the email notification. What do you think? If you agree I’ll implement it.

#43 Updated by intrigeri 2018-01-03 13:18:05

intrigeri wrote:
> So I propose that after running diffoscope, if diffoscope.txt is small enough (say, not more than 64KB), we cat it so its content ends up in the email notification.

Pushed something, let’s see how it goes.

#44 Updated by intrigeri 2018-01-04 05:42:19

intrigeri wrote:
> intrigeri wrote:
> > So I propose that after running diffoscope, if diffoscope.txt is small enough (say, not more than 64KB), we cat it so its content ends up in the email notification.
>
> Pushed something, let’s see how it goes.

Seems to work: https://jenkins.tails.boum.org/view/Tails_ISO/job/reproducibly_build_Tails_ISO_web-14997-explain-better-verification-and-failure/20/console. I’ve not seen the corresponding email notification, but given the ones I see include the end of the console output up to “Archiving artifacts”, we should be good :)