Skip to content

kvm/security_group: Make Security Group Python 3 compatible#3589

Merged
yadvr merged 2 commits into
apache:masterfrom
CLDIN:security-group-py3
Sep 26, 2019
Merged

kvm/security_group: Make Security Group Python 3 compatible#3589
yadvr merged 2 commits into
apache:masterfrom
CLDIN:security-group-py3

Conversation

@wido

@wido wido commented Sep 9, 2019

Copy link
Copy Markdown
Contributor

This script only runs on the KVM Hypervisors and these all support
Python 3.

As Python 2 is deprecated at the end of 2019 we need to fix these
scripts to work under Python 3.

CentOS 7, 8 and Ubuntu 16.04 and 18.04 all have Python 3 installed
by default.

Ubuntu 20.04 will no longer have Python 2 installed and therefor
this script needs to be modified to work with Python 3.

Signed-off-by: Wido den Hollander wido@widodh.nl

This script only runs on the KVM Hypervisors and these all support
Python 3.

As Python 2 is deprecated at the end of 2019 we need to fix these
scripts to work under Python 3.

CentOS 7, 8 and Ubuntu 16.04 and 18.04 all have Python 3 installed
by default.

Ubuntu 20.04 will no longer have Python 2 installed and therefor
this script needs to be modified to work with Python 3.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
@wido wido added this to the 4.14.0.0 milestone Sep 9, 2019
@GabrielBrascher

Copy link
Copy Markdown
Member

Just adding, for the record, that this is the first PR addressing the effort raised on issue #3195
Thanks for the PR @wido! I will be reviewing it and re-testing it.

@GabrielBrascher GabrielBrascher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and security group rules are applied as expected. LGTM based on code review and tests.

@yadvr

yadvr commented Sep 20, 2019

Copy link
Copy Markdown
Member

@wido @GabrielBrascher can you add dependency of python3 in packaging/centos7/cloud.spec to cloudstack-common and cloudstack-agent packages?

@yadvr yadvr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With el6 deprecated, this can be accepted now with ensuring that python3 is installed with cloudstack-common or cloudstack-agent packages.

@GabrielBrascher

Copy link
Copy Markdown
Member

@rhtyd updated adding the dependency of python3 in packaging/centos7/cloud.spec. Thanks!

Comment thread debian/control

Package: cloudstack-common
Architecture: all
Depends: ${misc:Depends}, ${python:Depends}, genisoimage, nfs-common, python-netaddr

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wido @GabrielBrascher Do we no longer need python-netaddr? (for all/any py scripts?)

Summary: CloudStack management server UI
Requires: java-1.8.0-openjdk
Requires: python
Requires: python3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want both python and python3 like the debian/control change @GabrielBrascher, please add a new Requires line

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhtyd ah sure. Updated it, thanks!

@yadvr

yadvr commented Sep 20, 2019

Copy link
Copy Markdown
Member

Thanks @GabrielBrascher left some minor remarks

@yadvr

yadvr commented Sep 20, 2019

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-285

@yadvr

yadvr commented Sep 20, 2019

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-376)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32972 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3589-t376-kvm-centos7.zip
Smoke tests completed. 77 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

@yadvr
yadvr merged commit 899eab6 into apache:master Sep 26, 2019
ustcweizhou pushed a commit to ustcweizhou/cloudstack that referenced this pull request Feb 28, 2020
)

* kvm/security_group: Make Security Group Python 3 compatible

This script only runs on the KVM Hypervisors and these all support
Python 3.

As Python 2 is deprecated at the end of 2019 we need to fix these
scripts to work under Python 3.

CentOS 7, 8 and Ubuntu 16.04 and 18.04 all have Python 3 installed
by default.

Ubuntu 20.04 will no longer have Python 2 installed and therefor
this script needs to be modified to work with Python 3.

Signed-off-by: Wido den Hollander <wido@widodh.nl>

* Add dependency of python3 in packaging/centos7/cloud.spec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants