Update test farm tests to stop using certbot-auto by ohemorange · Pull Request #8207 · certbot/certbot

ohemorange · 2020-08-13T21:01:11Z

Fixes #7955.
Fixes #7915.

auto_targets.yaml is just a copy of the targets.yaml from before any changes made in this PR.

bootstrap_os_packages.sh is a copy of certbot-auto with many, many things removed. It only uses python3, and the python3-devel.x86_64 package is added for RHEL7.

Python36SCL is enabled in the calling scripts by adding its location to PATH for centos6.

The updated test scripts also only use Python3 now. We still need set_python_envvars.sh for test_letsencrypt_auto_certonly_standalone.sh.

The change to targets.yaml is for test_sdists.sh on RHEL7 to install python3-devel, since we don't use site packages and need headers available to compile.

The commit history got a little messed up with which changes should go with which commit message, so it may be confusing to go commit by commit.

After this is merged, we should:

Update the release instructions to use auto_targets.yaml for test_leauto_upgrades.sh and test_letsencrypt_auto_certonly_standalone.sh.

bmw · 2020-08-14T18:58:13Z

I thought of an unfortunate interaction between this PR and #8206 dropping Python 3.5 support.

This PR switches some of our tests to use the Python 3 packages provided by the OS, but some of those versions of Python are 3.5. Running the test farm tests on the combination of these two PRs, the images for Debian 9 and Ubuntu 16.04 are affected. See https://dev.azure.com/certbot/certbot/_build/results?buildId=2480&view=results.

In the short term, I think we should continue with both PRs and stop running test_apache2.sh, test_sdists.sh, and test_tests.sh on Debian 9 and Ubuntu 16.04. One of the Debian 9 AMIs is our only ARM image so I think we should pretty quickly add back one of those on another OS, but I don't care if that happens in this PR or another one.

In the long term, I think we should continue not running test_sdists.sh and test_tests.sh on these OSes. They're just running our unit tests, those OS packages aren't seeing updates, and distro maintainers couldn't even update the packages if they wanted to since we no longer support their Python version.

It would be nice to run continue to run test_apache2.sh in the long term though. The best idea I have to accomplish that is to block the test_apache2.sh Azure build on the Certbot snap being built and to pass that file along to the EC2 instances. Another idea would be to install a version of Python from outside the OS package manager, maybe using something like pyenv. Neither approach is totally trivial though.

We could block one of our PRs on this if we want, but given how we have no work planned on our Apache plugin in the medium term, I think it's fine to go forward with these changes and add updating test_apache2.sh to our short/medium term priorities.

What do you think @ohemorange?

ohemorange · 2020-08-14T19:54:41Z

I think we should go ahead and merge the other PR, first thing. It might not be too bad to install python3.6 on debian 9 from source? I can take a quick shot at that. Otherwise the plan sounds good. I think I prefer upgrading python to switching around the test order.

bmw · 2020-08-14T20:05:02Z

I think we should go ahead and merge the other PR, first thing.

OK. I realized our website builds will also break with that change, but I'm working on that now.

It might not be too bad to install python3.6 on debian 9 from source? I can take a quick shot at that.

Thanks. Sorry I didn't anticipate the interactions here.

Couple things:

Any reason not to use a newer version of Python 3 to give us longer before needing to upgrade here?
Are you planning on trying this on Ubuntu 16.04 too?

It may be nice to always use an external Python version in test_apache2.sh but we could always do that later if we want.

Finally, in case it's useful, I got things to work on Ubuntu 20.04 with pyenv this morning. The annoying part was getting all of its dependencies installed which at least on that system were make git zlib1g-dev, in addition to Certbot's normal dependencies outside of Python. The full output of that experiment is at https://gist.github.com/704a1458bc5e684e18acbf18271be1bd in case that's useful.

ohemorange · 2020-08-14T20:22:46Z

Any reason not to use a newer version of Python 3 to give us longer before needing to upgrade here?

Probably not, though I can imagine theoretical other problems with deprecations if we go too new.

Are you planning on trying this on Ubuntu 16.04 too?

Depends on how debian 9 goes.

It may be nice to always use an external Python version in test_apache2.sh but we could always do that later if we want.

That might make this easier, actually.

Finally, in case it's useful, I got things to work on Ubuntu 20.04 with pyenv this morning. The annoying part was getting all of its dependencies installed which at least on that system were make git zlib1g-dev, in addition to Certbot's normal dependencies outside of Python. The full output of that experiment is at https://gist.github.com/704a1458bc5e684e18acbf18271be1bd in case that's useful.

That is useful, thanks! I'll give that a shot, getting the dependencies shouldn't be too bad.

ohemorange · 2020-08-15T00:32:19Z

~~I don't know why the arm64 ubuntu 20.04 isn't working, it's giving auth errors:~~

[Process-3 : client 1 ami-008680ee60f23c94b ubuntu20.04_arm64]
server ec2.Instance(id='i-05139a14238cca441') at 54.175.57.136
admin@54.175.57.136
ami-008680ee60f23c94b - ubuntu20.04_arm64 FAIL
Traceback (most recent call last):
  File "multitester.py", line 364, in test_client_process
    install_and_launch_certbot(cxn, instance, boulder_url, target)
  File "multitester.py", line 293, in install_and_launch_certbot
    local_repo_to_remote(cxn)
  File "multitester.py", line 266, in local_repo_to_remote
    cxn.put(local=local_path, remote='')
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/fabric/connection.py", line 786, in put
    return Transfer(self).put(*args, **kwargs)
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/fabric/transfer.py", line 233, in put
    self.sftp.getcwd() or self.sftp.normalize("."), remote
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/fabric/transfer.py", line 33, in sftp
    return self.connection.sftp()
  File "<decorator-gen-5>", line 2, in sftp
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/fabric/connection.py", line 29, in opens
    self.open()
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/fabric/connection.py", line 634, in open
    self.client.connect(**kwargs)
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/paramiko/client.py", line 426, in connect
    self._auth(
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/paramiko/client.py", line 749, in _auth
    raise saved_exception
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/paramiko/client.py", line 725, in _auth
    self._transport.auth_publickey(username, key)
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/paramiko/transport.py", line 1507, in auth_publickey
    return self.auth_handler.wait_for_response(my_event)
  File "/home/erica/certbot/venv3/lib/python3.8/site-packages/paramiko/auth_handler.py", line 250, in wait_for_response
    raise e
paramiko.ssh_exception.AuthenticationException: Authentication failed.

EDIT: nvm, I had the user wrong.

…to_upgrades.sh and test_letsencrypt_auto_certonly_standalone.sh

ohemorange · 2020-08-15T00:51:46Z

Tests are passing after merging in drop-py35 except for apache2 tests on a couple different arm64 versions I've tried (debian9, ubuntu20). It hangs at pyenv install 3.8.5. Will try to figure out why next week.

ohemorange · 2020-08-15T18:22:54Z

Tests are passing after merging in drop-py35 except for apache2 tests on a couple different arm64 versions I've tried (debian9, ubuntu20). It hangs at pyenv install 3.8.5. Will try to figure out why next week.

Oh wait. I just realized. If we just use a new enough OS for our arm rep, we absolutely do not need to solve this. woo.

ohemorange · 2020-08-15T19:15:00Z

Here's a passing test farm run with the deprecate py35 PR merged in: https://dev.azure.com/certbot/certbot/_build/results?buildId=2488&view=results

bmw

To flag this again as part of the review as well to help us remember, we'll need to update the release instructions after this lands.

bmw · 2020-08-17T19:01:55Z

tests/letstest/scripts/test_apache2.sh

-# instance, Fedora uses Python 3 and Python 2 is not installed.
-. tests/letstest/scripts/set_python_envvars.sh
-
-"$VENV_SCRIPT" -e acme[dev] -e certbot[dev,docs] -e certbot-apache


Whoops. It looks like most of the rest of this script has been deleted or commented out.

???????????????? oops

bmw · 2020-08-17T19:10:03Z

tests/letstest/targets.yaml

  #-----------------------------------------------------------------------------
  #Ubuntu
+  - ami: ami-0758470213bdd23b1
+    name: ubuntu20.04


Nice! This PR basically resolves #7915 too.

In this or another PR, I'd kind of like to add this amd64 image to apache2_targets.yaml but otherwise it's resolved in my book.

bmw · 2020-08-17T19:15:11Z

tests/letstest/targets.yaml

+    userdata: |
+      #cloud-config
+      runcmd:
+        - yum-config-manager --enable rhui-REGION-rhel-server-extras rhui-REGION-rhel-server-optional


Do you know if there's a potential race condition where our test farm test scripts start before this command is executed?

we rely on runcmd elsewhere so I had assumed not, but that could explain the one spurious failure in azure that I obviously couldn't check the logs on.

I'm not sure, but I suspect the other runcmd commands aren't really needed.

I personally think we should either convince ourselves we're not adding a race condition here or move this code to bootstrap_os_packages.sh. I think the latter shouldn't be too hard. I'm imagining something like:

BootstrapRpmPython3() { InitializeRPMCommonBase python_pkgs="python3 python3-devel " if ! $TOOL list python3-devel >/dev/null 2>&1; then yum-config-manager --enable rhui-REGION-rhel-server-extras rhui-REGION-rhel-server-optional python_pkgs="$python_pkgs python3-devel.x86_64 " fi BootstrapRpmCommonBase "$python_pkgs" }

but I didn't test that code.

EDIT: Flipped logic so it hopefully works.

I'm fine with just moving it there.

bmw · 2020-08-17T19:18:28Z

tests/letstest/targets.yaml

+      #cloud-config
+      runcmd:
+        - yum-config-manager --enable rhui-REGION-rhel-server-extras rhui-REGION-rhel-server-optional
+        - yum install -y python3-devel.x86_64


nit: Is this line needed with BootstrapRpmPython3 installing python3-devel? My understanding is the two package names are equivalent except this one hard codes the architecture while the other will just use the architecture of the machine its running on which should also be x86_64.

It does seem to be required, yes. It couldn't find python3-devel, which is why I added this here. Various q&as I found all do seem to indicate you specifically need the architecture on this machine, as well.

"elsewhere" being a few lines down, here: https://github.com/certbot/certbot/pull/8207/files#diff-776152681de94287606432a9922f03f8L81

It couldn't find python3-devel even after you enabled the necessary repos? That surprises me but trying to install the package twice doesn't hurt anything.

it's possibly that I didn't test that iteration! there were many things I tried. I'll double-check.

you're right, we don't need to specify the arch

ah, I've just remembered -- it's because python3-devel is neither necessary nor available on RHEL/centos8.

yum install python3-devel seems to work on CentOS/RHEL 8, but yum list python3-devel does not.

I think we can fix the list command by changing the command to if ! $TOOL list 'python3*-devel' >/dev/null 2>&1; then.

The problem seems to be that python3-devel is an alias for python36-devel with the install subcommand on CentOS/RHEL 8, but not the list subcommand.

oh, I like that better than how I have it.

ohemorange · 2020-08-17T20:43:47Z

Tests are failing after uncommenting apache2, oops, I'll look into that.

…tential race condition

bmw

LGTM!

Thanks for doing this. I think this is a big step towards dropping certbot-auto and Python 2.7 which I'm personally pretty excited about.

Can you take care of updating the release instructions?

ohemorange · 2020-08-18T17:07:16Z

Can you take care of updating the release instructions?

Yep, for sure.

Thanks for doing this. I think this is a big step towards dropping certbot-auto and Python 2.7 which I'm personally pretty excited about.

Me too!

ohemorange requested a review from bmw August 13, 2020 23:13

bmw mentioned this pull request Aug 14, 2020

Drop Python 3.5 support #8206

Merged

bmw added area: certbot-auto area: code health area: testing labels Aug 14, 2020

bmw self-assigned this Aug 14, 2020

ohemorange added 19 commits August 14, 2020 17:47

Create bootstrap script

c49cefb

Delete a whole bunch of the bootstrap script

20ea946

modify test_tests to use new script

98c3a50

put python version checking in back in

c25c898

add x

fba0aa7

call the venv creation from inside the bootstrap

58d637f

add targets back

1e8b371

modify test_apache2 to use new format

72241f5

shouldn't need virtualenv on rhel

72d1d73

readd targets

6fa506a

Update test_sdists to use new script

171fde5

move setting up venv back out of script so it's not run with sudo

ffdc6be

take venv3.py call out of bootstrap in all scripts

25cb278

add additional python3-devel pkg name

7bb8334

fix test_sdists

c3169e9

enable additional rhel7 repos

8f05571

clean up code and comments

14bcf86

Update tests and instructions to use auto_targets.yaml with test_leau…

7fa99e3

…to_upgrades.sh and test_letsencrypt_auto_certonly_standalone.sh

only install python3-devel.x86_64 for rhel7

a618bae

ohemorange added 8 commits August 14, 2020 17:47

Upgrade python version for debian in test_apache2.sh

3a43535

don't run test_tests or test_sdists on debian 9 or ubuntu 16.04

83e3e9b

Add 20.04 and 20.04 arm images to targets.yaml

c5c50fa

use pyenv to upgrade to python3.5

fe808a3

remove arm64 instance because it's having auth trouble

8721d56

correct pyenv usage on ubuntu

9cd2780

add arm64 target to targets.yaml

69289e4

replace debian 9 arm64 with ubuntu 20

7ab3d5f

ohemorange force-pushed the so-long-certbot-auto branch from 6079fca to 7ab3d5f Compare August 15, 2020 00:49

don't try to upgrade a perfectly good python version

71c31e5

Merge branch 'master' into so-long-certbot-auto

c3c39ba

bmw requested changes Aug 17, 2020

View reviewed changes

ohemorange added 2 commits August 17, 2020 13:15

let's just add ubuntu20 to apache2_targets while we're here

f701e69

uncomment test_apache2

45f30b5

ohemorange added 6 commits August 17, 2020 13:53

move adding python3-devel.x86_64 to bootstrap_os_packages to avoid po…

c90329c

…tential race condition

no need to specify the arch once extra rhel7 repos enabled

5ed0726

explicitly specify python3

a0e84e1

don't fail if we can't enable rhel7 extras

f6357dc

capture python36-devel as well

36c38cd

Merge branch 'master' into so-long-certbot-auto

3bbb91d

bmw approved these changes Aug 18, 2020

View reviewed changes

ohemorange merged commit acb6d34 into master Aug 18, 2020

ohemorange deleted the so-long-certbot-auto branch August 18, 2020 17:07

bmw mentioned this pull request Aug 19, 2020

Switch to a Debian 10 ARM image #8220

Closed

Uh oh!

Conversation

ohemorange commented Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bmw commented Aug 14, 2020

Uh oh!

ohemorange commented Aug 14, 2020

Uh oh!

bmw commented Aug 14, 2020

Uh oh!

ohemorange commented Aug 14, 2020

Uh oh!

ohemorange commented Aug 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ohemorange commented Aug 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ohemorange commented Aug 15, 2020

Uh oh!

ohemorange commented Aug 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bmw left a comment

Choose a reason for hiding this comment

Uh oh!

bmw Aug 17, 2020 • 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!

bmw Aug 17, 2020 • 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!

ohemorange commented Aug 17, 2020

Uh oh!

bmw left a comment

Choose a reason for hiding this comment

Uh oh!

ohemorange commented Aug 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ohemorange commented Aug 13, 2020 •

edited

Loading

ohemorange commented Aug 15, 2020 •

edited

Loading

ohemorange commented Aug 15, 2020 •

edited

Loading

ohemorange commented Aug 15, 2020 •

edited

Loading

bmw Aug 17, 2020 •

edited

Loading

bmw Aug 17, 2020 •

edited

Loading