Skip to content

Conversation

@cl-k-takahashi
Copy link
Contributor

@cl-k-takahashi cl-k-takahashi commented Aug 1, 2019

Signed-off-by: Kai Takahashi k-takahashi@creationline.com

Description

Extract systemvm.iso using bsdtar in injectkeys.sh if available.
With bsdtar, injectkeys.sh doesn't need to mount systemvm.iso and works even on unprivileged containers.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

Checked that a new key is injected to systemvm.iso by running injectkeys.sh manually.

@rohityadavcloud
Copy link
Member

@cl-k-takahashi how about we add bsdtar as the dependency for both debian/rpm packages? That way we can enforce CloudStack to use bsdtar?

@rohityadavcloud rohityadavcloud added this to the 4.13.1.0 milestone Aug 1, 2019
@cl-k-takahashi
Copy link
Contributor Author

@cl-k-takahashi how about we add bsdtar as the dependency for both debian/rpm packages? That way we can enforce CloudStack to use bsdtar?

@rhtyd I agree. Should I remove all 'mount' command calls from injectkeys.sh?

@cl-k-takahashi
Copy link
Contributor Author

While bsdtar is available in Ubuntu and CentOS 7 official repository, it's in EPEL in CentOS 6.

@cl-k-takahashi
Copy link
Contributor Author

I added bsdtar as the dependency for rpm(CentOS7)/debian packages.
The cloud.spec for CentOS 6 is untouched.

@rohityadavcloud
Copy link
Member

@cl-k-takahashi I've moved the milestone to 4.14, in which we'll remove support for CentOS6 (reference https://markmail.org/message/eenfpyya2wfgd2gw), then your PR will be acceptable. Thanks.

@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-271

Signed-off-by: Kai Takahashi <k-takahashi@creationline.com>
bsdtar can extract iso images without mounting.

Signed-off-by: Kai Takahashi <k-takahashi@creationline.com>
@cl-k-takahashi
Copy link
Contributor Author

Solved a conflict: A dependency removed from debian/control: python-netaddr was removed from the debian dependency in master branch.

@cl-k-takahashi
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

@rohityadavcloud
Copy link
Member

@blueorangutan test centos7 vmware-65u2

@blueorangutan
Copy link

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

@blueorangutan
Copy link

Trillian test result (tid-389)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48864 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3536-t389-vmware-65u2.zip
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 75 look OK, 2 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_00_deploy_vm_root_resize Failure 320.03 test_deploy_vm_root_resize.py
test_05_rvpc_multi_tiers Failure 598.88 test_vpc_redundant.py
test_05_rvpc_multi_tiers Error 641.18 test_vpc_redundant.py

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

@rohityadavcloud
Copy link
Member

@blueorangutan test centos7 vmware-65u2

@blueorangutan
Copy link

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

@andrijapanicsb
Copy link
Contributor

@blueorangutan test centos7 vmware-65u2

@blueorangutan
Copy link

@andrijapanicsb a Trillian-Jenkins test job (centos7 mgmt + vmware-65u2) has been kicked to run smoke tests

Copy link
Contributor

@andrijapanicsb andrijapanicsb left a comment

Choose a reason for hiding this comment

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

LGTM

tested manually with custom keys (VMware).

@andrijapanicsb
Copy link
Contributor

@DaanHoogland kindly take a look if you can LGTM this one.

@blueorangutan
Copy link

Trillian test result (tid-684)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 50741 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3536-t684-vmware-65u2.zip
Smoke tests completed. 77 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

@DaanHoogland
Copy link
Contributor

code looks good, nice cleanup. It does change the logic concerning file permissions, though. The logic looks fine and *should {tm} work on all platforms ;), @andrijapanicsb. If you tested it, I'm fine with it.

@andrijapanicsb
Copy link
Contributor

Yes, manually tested by injecting custom keys manually running the same command/script and verified the modified pub key present in SSVM.

@DaanHoogland
Copy link
Contributor

👍 master or first to 4.13?

@andrijapanicsb
Copy link
Contributor

I've tested master, but this is a change, not bugfix, so I say only master @DaanHoogland?

@DaanHoogland
Copy link
Contributor

ok, go ahead, @andrijapanicsb ! looks good

@andrijapanicsb
Copy link
Contributor

Mergng based on 2 x reviews and manual testing and regression testing (no failures).

@andrijapanicsb andrijapanicsb merged commit 8a55c93 into apache:master Jan 6, 2020
@cl-k-takahashi cl-k-takahashi deleted the patch-injectkeys-bsdtar branch January 6, 2020 13:00
anuragaw pushed a commit to shapeblue/cloudstack that referenced this pull request Jan 10, 2020
andrijapanicsb pushed a commit that referenced this pull request Jan 13, 2020
@DaanHoogland
Copy link
Contributor

@cl-k-takahashi Can you please see the comments in #3800 and resubmit this? I don't think the change will be big, but it a very specific error during upgrades that needs addressing. Sorry for the invconvenience.

Spaceman1984 pushed a commit to shapeblue/cloudstack that referenced this pull request Jan 17, 2020
ustcweizhou pushed a commit to ustcweizhou/cloudstack that referenced this pull request Feb 28, 2020
* Extract systemvm.iso using bsdtar if available.

Signed-off-by: Kai Takahashi <k-takahashi@creationline.com>

* New dependency for CentOS 7 and Debian: bsdtar

bsdtar can extract iso images without mounting.

Signed-off-by: Kai Takahashi <k-takahashi@creationline.com>

* Remove all 'mount' and 'umount' command call(s).

Signed-off-by: Kai Takahashi <k-takahashi@creationline.com>
ustcweizhou pushed a commit to ustcweizhou/cloudstack that referenced this pull request Feb 28, 2020
cl-k-takahashi added a commit to cl-k-takahashi/cloudstack that referenced this pull request Apr 2, 2020
Signed-off-by: Kai Takahashi <k-takahashi@creationline.com>
cl-k-takahashi added a commit to cl-k-takahashi/cloudstack that referenced this pull request Apr 2, 2020
Signed-off-by: Kai Takahashi <k-takahashi@creationline.com>
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.

6 participants