Code review comment for autopkgtest-cloud:more-fail-strings

Revision history for this message
Paride Legovini (paride) wrote :

The change is OK but I'm not entirely happy with it as FAIL_PKG_STRINGS already has a "Temporary failure resolving" entry, see:

{'systemd*': ['timed out waiting for testbed to reboot',
              'Timed out on waiting for ssh connection',
              'Temporary failure resolving',
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^

which is exactly the error which appears in the log excerpt that Brian posted a couple of comments above:

Err:1 https://ppa.launchpadcontent.net/enr0n/systemd-broken/ubuntu kinetic/main systemd 251.4-1ubuntu1~ppa6 (tar)
  Temporary failure resolving 'ppa.launchpadcontent.net'

so this failure case should be already covered. I dug up where that line was added and here is it:

https://git.launchpad.net/autopkgtest-cloud/commit/?id=625889eb9b6a7a8075e20635df4a6c59fb935eb0

note the commit message:

  In case systemd itself breaks DNS;
  we don't want this to be a tmpfail loop.

Iain was trying to avoid the same retry loop we're trying to tackle here, but for some reason that's not working, and I can't see why.

(Also as I understand it FAIL_PKG_STRINGS is meant to cover *specific* breakage caused by the package (see the comment that describes its purpose), while here we're adding a somehow catchall apt error to the list, which is not specific to systemd.)

I don't want to block merging, but if you can think of why 'Temporary failure resolving' doesn't count as a non-temporary failure for systemd then let's fix this in a more proper way.

review: Needs Information

« Back to merge proposal