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.

Sign up for GitHub

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

Expose serial number and bus type via GuestOsInfo #10970

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Expose serial number and bus type via GuestOsInfo #10970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e407cd
Expose serial number and bus type via virtctl
alromeros Jan 5, 2024
693df1e
Add unit tests
alromeros Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/api-rule-violations-known.list
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ API rule violation: list_type_missing,kubevirt.io/api/core/v1,KubeVirtConfigurat
API rule violation: list_type_missing,kubevirt.io/api/core/v1,KubeVirtConfiguration,SupportedGuestAgentVersions
API rule violation: list_type_missing,kubevirt.io/api/core/v1,KubeVirtStatus,Conditions
API rule violation: list_type_missing,kubevirt.io/api/core/v1,NodePlacement,Tolerations
API rule violation: list_type_missing,kubevirt.io/api/core/v1,VirtualMachineInstanceFileSystem,Disk
API rule violation: list_type_missing,kubevirt.io/api/core/v1,VirtualMachineInstanceFileSystemInfo,Filesystems
API rule violation: list_type_missing,kubevirt.io/api/core/v1,VirtualMachineInstanceGuestAgentInfo,UserList
API rule violation: list_type_missing,kubevirt.io/api/core/v1,VirtualMachineInstanceMigrationStatus,Conditions
Expand Down Expand Up @@ -253,6 +254,7 @@ API rule violation: names_match,kubevirt.io/api/core/v1,LunTarget,ReadOnly
API rule violation: names_match,kubevirt.io/api/core/v1,NetworkConfiguration,NetworkInterface
API rule violation: names_match,kubevirt.io/api/core/v1,PITTimer,Enabled
API rule violation: names_match,kubevirt.io/api/core/v1,RTCTimer,Enabled
API rule violation: names_match,kubevirt.io/api/core/v1,VirtualMachineInstanceFileSystemDisk,BusType
API rule violation: names_match,kubevirt.io/api/core/v1,VirtualMachineInstanceFileSystemInfo,Filesystems
API rule violation: names_match,kubevirt.io/api/core/v1,VirtualMachineInstanceGuestAgentInfo,GAVersion
API rule violation: names_match,kubevirt.io/api/core/v1,VirtualMachineInstanceGuestOSInfo,VersionID
Expand Down
2 changes: 2 additions & 0 deletions api/api-rule-violations.list
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ API rule violation: list_type_missing,kubevirt.io/api/core/v1,KubeVirtConfigurat
API rule violation: list_type_missing,kubevirt.io/api/core/v1,KubeVirtConfiguration,SupportedGuestAgentVersions
API rule violation: list_type_missing,kubevirt.io/api/core/v1,KubeVirtStatus,Conditions
API rule violation: list_type_missing,kubevirt.io/api/core/v1,NodePlacement,Tolerations
API rule violation: list_type_missing,kubevirt.io/api/core/v1,VirtualMachineInstanceFileSystem,Disk
API rule violation: list_type_missing,kubevirt.io/api/core/v1,VirtualMachineInstanceFileSystemInfo,Filesystems
API rule violation: list_type_missing,kubevirt.io/api/core/v1,VirtualMachineInstanceGuestAgentInfo,UserList
API rule violation: list_type_missing,kubevirt.io/api/core/v1,VirtualMachineInstanceMigrationStatus,Conditions
Expand Down Expand Up @@ -253,6 +254,7 @@ API rule violation: names_match,kubevirt.io/api/core/v1,LunTarget,ReadOnly
API rule violation: names_match,kubevirt.io/api/core/v1,NetworkConfiguration,NetworkInterface
API rule violation: names_match,kubevirt.io/api/core/v1,PITTimer,Enabled
API rule violation: names_match,kubevirt.io/api/core/v1,RTCTimer,Enabled
API rule violation: names_match,kubevirt.io/api/core/v1,VirtualMachineInstanceFileSystemDisk,BusType
API rule violation: names_match,kubevirt.io/api/core/v1,VirtualMachineInstanceFileSystemInfo,Filesystems
API rule violation: names_match,kubevirt.io/api/core/v1,VirtualMachineInstanceGuestAgentInfo,GAVersion
API rule violation: names_match,kubevirt.io/api/core/v1,VirtualMachineInstanceGuestOSInfo,VersionID
Expand Down
25 changes: 25 additions & 0 deletions api/openapi-spec/swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19720,6 +19720,13 @@
"totalBytes"
],
"properties": {
"disk": {
"type": "array",
"items": {
"default": {},
"$ref": "#/definitions/v1.VirtualMachineInstanceFileSystemDisk"
}
},
"diskName": {
"type": "string",
"default": ""
Expand All @@ -19744,6 +19751,24 @@
}
}
},
"v1.VirtualMachineInstanceFileSystemDisk": {
"description": "VirtualMachineInstanceFileSystemDisk represents the guest os FS disks",
"type": "object",
"required": [
"serial",
"bus-type"
],
"properties": {
"bus-type": {
"type": "string",
"default": ""
},
"serial": {
"type": "string",
"default": ""
}
}
},
"v1.VirtualMachineInstanceFileSystemInfo": {
"description": "VirtualMachineInstanceFileSystemInfo represents information regarding single guest os filesystem",
"type": "object",
Expand Down
1 change: 1 addition & 0 deletions pkg/virt-launcher/virtwrap/agent-poller/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ go_test(
embed = [":go_default_library"],
deps = [
"//pkg/virt-launcher/virtwrap/api:go_default_library",
"//staging/src/kubevirt.io/api/core/v1:go_default_library",
"//staging/src/kubevirt.io/client-go/testutils:go_default_library",
"//vendor/github.com/onsi/ginkgo/v2:go_default_library",
"//vendor/github.com/onsi/gomega:go_default_library",
Expand Down
12 changes: 7 additions & 5 deletions pkg/virt-launcher/virtwrap/agent-poller/agent_parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,12 @@ type User struct {

// Filesystem of the host
type Filesystem struct {
Name string `json:"name"`
Mountpoint string `json:"mountpoint"`
Type string `json:"type"`
UsedBytes int `json:"used-bytes,omitempty"`
TotalBytes int `json:"total-bytes,omitempty"`
Name string `json:"name"`
Mountpoint string `json:"mountpoint"`
Type string `json:"type"`
UsedBytes int `json:"used-bytes,omitempty"`
TotalBytes int `json:"total-bytes,omitempty"`
Disk []v1.VirtualMachineInstanceFileSystemDisk `json:"disk,omitempty"`
}

// AgentInfo from the guest VM serves the purpose
Expand Down Expand Up @@ -196,6 +197,7 @@ func parseFilesystem(agentReply string) ([]api.Filesystem, error) {
Type: fs.Type,
TotalBytes: fs.TotalBytes,
UsedBytes: fs.UsedBytes,
Disk: fs.Disk,
Copy link
Contributor

@enp0s3 enp0s3 Jan 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is it safe to just assign it like that w/o iterating and running append to the list you create with a make?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think it's safe to just assign slices

})
}

Expand Down
16 changes: 15 additions & 1 deletion pkg/virt-launcher/virtwrap/agent-poller/agent_parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

v1 "kubevirt.io/api/core/v1"

"kubevirt.io/kubevirt/pkg/virt-launcher/virtwrap/api"
)

Expand Down Expand Up @@ -268,7 +270,13 @@ var _ = Describe("Qemu agent poller", func() {
"mountpoint":"/",
"type":"ext",
"total-bytes":99999,
"used-bytes":33333
"used-bytes":33333,
"disk":[
{
"serial":"testserial-1234",
"bus-type":"scsi"
}
]
}
]
}`
Expand All @@ -280,6 +288,12 @@ var _ = Describe("Qemu agent poller", func() {
Type: "ext",
TotalBytes: 99999,
UsedBytes: 33333,
Disk: []v1.VirtualMachineInstanceFileSystemDisk{
{
Serial: "testserial-1234",
BusType: "scsi",
},
},
},
}
Expect(parseFilesystem(jsonInput)).To(Equal(expectedFilesystem))
Expand Down
6 changes: 6 additions & 0 deletions pkg/virt-launcher/virtwrap/api/deepcopy_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/virt-launcher/virtwrap/api/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ type Filesystem struct {
Type string
UsedBytes int
TotalBytes int
Disk []v1.VirtualMachineInstanceFileSystemDisk
}

type User struct {
Expand Down
6 changes: 6 additions & 0 deletions pkg/virt-launcher/virtwrap/cmd-server/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,12 @@ var _ = Describe("Virt remote commands", func() {
FileSystemType: "EXT4",
UsedBytes: 3333,
TotalBytes: 9999,
Disk: []v1.VirtualMachineInstanceFileSystemDisk{
{
BusType: "scsi",
Serial: "testserial-1234",
},
},
},
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/virt-launcher/virtwrap/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2029,6 +2029,7 @@ func (l *LibvirtDomainManager) GetGuestInfo() v1.VirtualMachineInstanceGuestAgen
FileSystemType: fs.Type,
UsedBytes: fs.UsedBytes,
TotalBytes: fs.TotalBytes,
Disk: fs.Disk,
})
}

Expand Down Expand Up @@ -2073,6 +2074,7 @@ func (l *LibvirtDomainManager) GetFilesystems() []v1.VirtualMachineInstanceFileS
FileSystemType: fs.Type,
UsedBytes: fs.UsedBytes,
TotalBytes: fs.TotalBytes,
Disk: fs.Disk,
})
}

Expand Down
30 changes: 30 additions & 0 deletions pkg/virt-launcher/virtwrap/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2208,6 +2208,12 @@ var _ = Describe("Manager", func() {
Type: "fs",
UsedBytes: 0,
TotalBytes: 0,
Disk: []v1.VirtualMachineInstanceFileSystemDisk{
{
BusType: "scsi",
Serial: "testserial-1234",
},
},
},
})

Expand All @@ -2228,6 +2234,12 @@ var _ = Describe("Manager", func() {
FileSystemType: "fs",
UsedBytes: 0,
TotalBytes: 0,
Disk: []v1.VirtualMachineInstanceFileSystemDisk{
{
BusType: "scsi",
Serial: "testserial-1234",
},
},
}))
})

Expand Down Expand Up @@ -2259,6 +2271,12 @@ var _ = Describe("Manager", func() {
Type: "fs",
UsedBytes: 0,
TotalBytes: 0,
Disk: []v1.VirtualMachineInstanceFileSystemDisk{
{
BusType: "scsi",
Serial: "testserial-1234",
},
},
},
})

Expand All @@ -2280,6 +2298,12 @@ var _ = Describe("Manager", func() {
Type: "fs",
UsedBytes: 0,
TotalBytes: 0,
Disk: []v1.VirtualMachineInstanceFileSystemDisk{
{
BusType: "scsi",
Serial: "testserial-1234",
},
},
},
})

Expand Down Expand Up @@ -2320,6 +2344,12 @@ var _ = Describe("Manager", func() {
Type: "fs",
UsedBytes: 0,
TotalBytes: 0,
Disk: []v1.VirtualMachineInstanceFileSystemDisk{
{
BusType: "scsi",
Serial: "testserial-1234",
},
},
},
})

Expand Down
29 changes: 27 additions & 2 deletions staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 12 additions & 5 deletions staging/src/kubevirt.io/api/core/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2215,13 +2215,20 @@ type VirtualMachineInstanceFileSystemList struct {
Items []VirtualMachineInstanceFileSystem `json:"items"`
}

// VirtualMachineInstanceFileSystemDisk represents the guest os FS disks
type VirtualMachineInstanceFileSystemDisk struct {
Serial string `json:"serial"`
Copy link
Member

Choose a reason for hiding this comment

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

Missing omitempty

Copy link
Member

Choose a reason for hiding this comment

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

@alromeros is this ever expected to be empty? I would be pretty surprised

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think BusType will never be empty, but Serial is optional: https://qemu-project.gitlab.io/qemu/interop/qemu-ga-ref.html#qapidoc-146.

BusType string `json:"bus-type"`
Copy link
Member

Choose a reason for hiding this comment

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

This does break convention, it should be json:"busType".

Copy link
Contributor

Choose a reason for hiding this comment

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

@xpivarc Thanks for the heads up! this check must be automated somehow, it is something hard to remember for the long term.

Copy link
Member

Choose a reason for hiding this comment

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

we can make the make generate fail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make generate failed but I added an exception to allow this formatting so we don't need an intermediary type:

API rule violation: names_match,kubevirt.io/api/core/v1,VirtualMachineInstanceFileSystemDisk,BusType

I will fix it to avoid breaking the convention.

Copy link
Member

Choose a reason for hiding this comment

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

I assumed we wanted to expose with exact naming naming scheme as libvirt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah me too. It prevents adding some unnecessary code, but I guess it's odd to only add the exception to BusType.

}

// VirtualMachineInstanceFileSystem represents guest os disk
type VirtualMachineInstanceFileSystem struct {
DiskName string `json:"diskName"`
MountPoint string `json:"mountPoint"`
FileSystemType string `json:"fileSystemType"`
UsedBytes int `json:"usedBytes"`
TotalBytes int `json:"totalBytes"`
DiskName string `json:"diskName"`
MountPoint string `json:"mountPoint"`
FileSystemType string `json:"fileSystemType"`
UsedBytes int `json:"usedBytes"`
TotalBytes int `json:"totalBytes"`
Disk []VirtualMachineInstanceFileSystemDisk `json:"disk,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Why is this a slice?

Copy link
Contributor

Choose a reason for hiding this comment

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

I got the explanation here

Copy link
Member

Choose a reason for hiding this comment

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

That doesn't explain the question, @alromeros can you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are storing here the guest agent response to the guest-get-fsinfo command. The command returns an array of disks, so I'm using one: https://qemu-project.gitlab.io/qemu/interop/qemu-ga-ref.html#qapidoc-161.

}

// FreezeUnfreezeTimeout represent the time unfreeze will be triggered if guest was not unfrozen by unfreeze command
Expand Down
6 changes: 6 additions & 0 deletions staging/src/kubevirt.io/api/core/v1/types_swagger_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading