Skip to content

config drive - send user-defined vm name as host name, not uuid#2637

Closed
ernjvr wants to merge 2 commits into
apache:4.11from
ernjvr:config-drive-vmname-as-hostname
Closed

config drive - send user-defined vm name as host name, not uuid#2637
ernjvr wants to merge 2 commits into
apache:4.11from
ernjvr:config-drive-vmname-as-hostname

Conversation

@ernjvr
Copy link
Copy Markdown
Contributor

@ernjvr ernjvr commented May 11, 2018

Description

For config drive to be Openstack compliant, in the openstack/latest/meta_data.json, we need the ‘name’ to be the name that the user sets as the VM name and the ‘hostname’ to be “ . ” ie. the fqdn.
Cherry picked a commit that fixes this: 105b3fe

Create an L2 network offering with user data checkbox selected to pass user data and metadata to VMs via config Drive. Create an L2 network and create a vm with userdata. Launch the VM. Ssh to the vm instance. Mount the config drive and verify that the openstack/latest/meta_data.json file contains the name that the user set as the VM name and the ‘hostname’ as “ . ” ie. the fqdn.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • [x ] 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)

GitHub Issue/PRs

Fixes #2615

Screenshots (if appropriate):

How Has This Been Tested?

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
    Testing
  • I have added tests to cover my changes.
  • All relevant new and existing integration tests have passed.
  • A full integration testsuite with all test that can run on my environment has passed.

@ernjvr
Copy link
Copy Markdown
Contributor Author

ernjvr commented May 11, 2018

@PaulAngus @DaanHoogland @rhtyd please review

@yadvr yadvr requested a review from fmaximus May 11, 2018 11:07
@yadvr yadvr added this to the 4.11.1.0 milestone May 11, 2018
@yadvr yadvr requested review from DaanHoogland and PaulAngus May 11, 2018 11:07
@ernjvr
Copy link
Copy Markdown
Contributor Author

ernjvr commented May 11, 2018 •
edited
Loading

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 11, 2018

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

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 11, 2018

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

DaanHoogland
DaanHoogland approved these changes May 11, 2018
View reviewed changes
Comment thread server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java Outdated
when(virtualMachine.getDataCenterId()).thenReturn(DATACENTERID);
when(virtualMachine.getInstanceName()).thenReturn(VMINSTANCENAME);
when(virtualMachine.getUserData()).thenReturn(VMUSERDATA);
when(virtualMachine.getHostName()).thenReturn("vm-hostname");
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland May 11, 2018

Choose a reason for hiding this comment

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

can you externalise this string (like the ones above)

yadvr
yadvr approved these changes May 11, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

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

LGTM, subject to testing.

@borisstoyanov
Copy link
Copy Markdown
Contributor

borisstoyanov commented May 14, 2018

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 14, 2018

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

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 14, 2018

Trillian test result (tid-2671)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 27134 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2637-t2671-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_public_ip_range.py
Intermitten failure detected: /marvin/tests/smoke/test_templates.py
Intermitten failure detected: /marvin/tests/smoke/test_usage.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 62 look OK, 5 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_03_vpc_privategw_restart_vpc_cleanup Error 113.29 test_privategw_acl.py
test_04_extract_template Failure 128.34 test_templates.py
ContextSuite context=TestISOUsage>:setup Error 0.00 test_usage.py
test_06_download_detached_volume Failure 136.53 test_volumes.py
test_hostha_enable_ha_when_host_in_maintenance Error 3.50 test_hostha_kvm.py

borisstoyanov
borisstoyanov approved these changes May 14, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@borisstoyanov borisstoyanov left a comment

Choose a reason for hiding this comment

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

LGTM

yadvr
yadvr requested changes May 15, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

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

!LGTM, failed manual test, internal instance name is still passed see:
screenshot from 2018-05-15 21-15-41

@yadvr
Copy link
Copy Markdown
Member

yadvr commented May 15, 2018

I had to port changes from another PR, I've them converged and tested here: https://github.com/apache/cloudstack/pull/2645/files
Closing this.

@yadvr yadvr closed this May 15, 2018
@yadvr yadvr removed this from the 4.11.1.0 milestone May 15, 2018
@ernjvr ernjvr deleted the config-drive-vmname-as-hostname branch May 16, 2018 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yadvr yadvr yadvr requested changes

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@borisstoyanov borisstoyanov borisstoyanov approved these changes

@fmaximus fmaximus Awaiting requested review from fmaximus

@PaulAngus PaulAngus Awaiting requested review from PaulAngus

+1 more reviewer

@nitin-maharana nitin-maharana nitin-maharana approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ernjvr @blueorangutan @borisstoyanov @yadvr @DaanHoogland @nitin-maharana