Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass system uuid as domain uuid and in sysinfo #714

Merged
merged 1 commit into from
Feb 6, 2019
Merged

Conversation

jellonek
Copy link
Contributor

@jellonek jellonek commented Jul 10, 2018

This change is Reviewable

@jellonek
Copy link
Contributor Author

First of commits is also separated to #713

@stale
Copy link

stale bot commented Aug 28, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Aug 28, 2018
@pigmej pigmej added the wip label Aug 31, 2018
@stale stale bot removed the wontfix label Aug 31, 2018
@jellonek jellonek changed the title [DNM] [PoC] Pass system uuid as domain uuid and in sysinfo Pass system uuid as domain uuid and in sysinfo Jan 14, 2019
@jellonek jellonek force-pushed the jell/systemuuidpoc branch 3 times, most recently from 097e4f5 to 1ac4126 Compare January 14, 2019 16:04
@CLAassistant
Copy link

CLAassistant commented Jan 14, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@ivan4th ivan4th left a comment

Choose a reason for hiding this comment

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

👍 for including the tests!

Reviewed 7 of 10 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @jellonek)


docs/resource_managment.md, line 23 at r1 (raw file):

As Kubelet uses cAdvisor to collect metrics about running containers and Virtlet doesn't create container per each VM, and instead spawns VMs inside Virtlet container. This leads to all the resource usage being lumped together and ascribed to Virtlet pod.

## Setting a particular value for System UUID

Using fixed SMBIOS UUID


docs/resource_managment.md, line 24 at r1 (raw file):

## Setting a particular value for System UUID
By default each VM pod will have autogenerated value for System UUID.  Some images can expect it to have a predictible value e.g.

By default, VM pods use autogenerated SMBIOS UUID values. Some images may expect it to have a fixed value, for example, due to software license requirements. In such cases, the value of SMBIOS UUID can be passed using VirtletSystemUUID annotation.


docs/resource_managment.md, line 34 at r1 (raw file):

Note: single instance of Virtlet can handle only single instance of VM pod with a particular value for System UUID setting.  That means
there can not be multiple VM pods on the same node with the same value of System UUID.  In the same time VM pods with the same value
of SystemUUID can coexist in the same cluster when started (e.g. using DaemonSet) once per node.

Note: Virtlet can't handle multiple VMs with the same SMBIOS UUID on the same node. There can be multiple VM pods with the same SMBIOS UUIDs residing on different nodes in the cluster, though.


pkg/metadata/types/annotations.go, line 121 at r1 (raw file):

	// SystemUUID specifies particular uuid to use as system UUID
	// in domain definition.  If not set - new one will be generated
	// basing on pod id.

SystemUUID specifies fixed SMBIOS UUID to be used for the domain.
If not set, the SMBIOS UUID will be automatically generated from the Pod ID.


pkg/metadata/types/annotations.go, line 315 at r1 (raw file):

		var err error
		if va.SystemUUID, err = uuid.ParseHex(systemUUIDStr); err != nil {
			return fmt.Errorf("failed to parse %q as system uuid: %v", systemUUIDStr, err)

as a UUID


tests/e2e/systemuuid_test.go, line 48 at r1 (raw file):

	scheduleWaitSSH(&vm, &ssh)

	It("Should have particular System UUID set [Conformance]", func() {

Should have the specified SMBIOS UUID set [Conformance]

Copy link
Contributor Author

@jellonek jellonek left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @ivan4th)


docs/resource_managment.md, line 23 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Using fixed SMBIOS UUID

Done.


docs/resource_managment.md, line 24 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

By default, VM pods use autogenerated SMBIOS UUID values. Some images may expect it to have a fixed value, for example, due to software license requirements. In such cases, the value of SMBIOS UUID can be passed using VirtletSystemUUID annotation.

Done.


docs/resource_managment.md, line 34 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Note: Virtlet can't handle multiple VMs with the same SMBIOS UUID on the same node. There can be multiple VM pods with the same SMBIOS UUIDs residing on different nodes in the cluster, though.

Done.


pkg/metadata/types/annotations.go, line 121 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

SystemUUID specifies fixed SMBIOS UUID to be used for the domain.
If not set, the SMBIOS UUID will be automatically generated from the Pod ID.

Done.


pkg/metadata/types/annotations.go, line 315 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

as a UUID

Done.


tests/e2e/systemuuid_test.go, line 48 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Should have the specified SMBIOS UUID set [Conformance]

Done.

Copy link
Contributor

@ivan4th ivan4th left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r2.
Reviewable status: 1 change requests, 0 of 1 approvals obtained

@ivan4th ivan4th merged commit ffad578 into master Feb 6, 2019
@ivan4th ivan4th deleted the jell/systemuuidpoc branch February 6, 2019 10:03
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