Nova Hypervisors API by spjmurray · Pull Request #178 · gophercloud/gophercloud

spjmurray · 2016-11-03T15:08:09Z

Adds in support to list all available hypervisors (detailed) for applications
such as monitoring and capacity planning.

API: https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/hypervisors.py#L46

Pay particular attention to line 71 where the horrible cpu_info hack is introduced

coveralls · 2016-11-03T15:12:20Z

Coverage increased (+0.02%) to 68.397% when pulling 10f6a0a on spjmurray:compute_hypervisors_list into 22bf82b on gophercloud:master.

jtopjian · 2016-11-03T15:17:21Z

@spjmurray Nice! I'd love to see this added.

Can you open a corresponding Issue per the style guide? This helps separate feature discussion vs code review discussion.

For the code itself, one question I have is whether or not this should go under directly under the compute/v2 package or compute/v2/extensions. The previous API docs had this listed as an extension, but now everything is bundled under compute. @jrperritt: thoughts?

spjmurray · 2016-11-03T15:33:04Z

Sure thing @jtopjian. Reads Also FYI the end goal of this is related to influxdata/telegraf#243

jtopjian · 2016-11-03T15:34:34Z

@spjmurray This just might make me revisit InfluxDB now...

spjmurray · 2016-11-03T15:39:35Z

@jtopjian Here's some more fun InfluxDB related things I've done to really stoke a fire ;p http://www.datacentred.co.uk/blog/icinga-2-5-released/ http://www.datacentred.co.uk/blog/ceph-monitoring-telegraf-grafana/

coveralls · 2016-11-03T15:48:55Z

Coverage increased (+0.02%) to 68.397% when pulling e4c6181 on spjmurray:compute_hypervisors_list into 22bf82b on gophercloud:master.

jrperritt · 2017-01-12T03:53:51Z

@spjmurray is this still correctly labeled WIP or is it ready for review?

Adds in support to list all available hypervisors (detailed) for applications such as monitoring and capacity planning. For gophercloud#179

spjmurray · 2017-02-20T11:50:46Z

Apologies @jtopjian this got buried in my inbox and forgotten about. Yes this is ready for review, I've stripped the WIP out of the pull request and the commit message.

coveralls · 2017-02-20T11:51:03Z

Coverage increased (+0.008%) to 69.472% when pulling 530d34d on spjmurray:compute_hypervisors_list into f47ca3a on gophercloud:master.

jrperritt

I've made some comments (mostly stylistic). If you'd prefer, I can submit this code as a PR, which makes most of the changes I mentioned on top of your commit.
The style guide should mention all the changes needed.

jrperritt · 2017-03-10T00:43:09Z

+	Arch     string   `json:"arch"`
+	Model    string   `json:"model"`
+	Features []string `json:"features"`
+	Topology `json:"topology"`


Let's name this field (Topology).

jrperritt · 2017-03-10T00:44:34Z

+
+type Hypervisor struct {
+	// A structure that contains cpu information like arch, model, vendor, features and topology
+	CpuInfo `json:"cpu_info"`


Let's change this to:

CPUInfo CPUInfo `json:"-"`

jrperritt · 2017-03-10T00:45:50Z

+	// The number of running vms on this hypervisor
+	RunningVMs int `json:"running_vms"`
+	// The hypervisor service object
+	Service `json:"service"`


Let's name this field (Service).

jrperritt · 2017-03-10T00:46:23Z

+	// The hypervisor type
+	HypervisorType string `json:"hypervisor_type"`
+	// The hypervisor version
+	HypervisorVersion int `json:"hypervisor_version"`


Let's change this to:

HypervisorVersion int `json:"-"`

jrperritt · 2017-03-10T00:47:34Z

+	// Newer versions pass the CPU into around as the correct types, this just needs
+	// converting and copying into place. Older versions pass CPU info around as a string
+	// and can simply be unmarshalled by the json parser
+	switch t := hypervisor.CpuInfo.(type) {


Is there any reason this couldn't be simplified to this:
master...jrperritt:compute_hypervisors_list#diff-cf3e97a140d10e9f8115943de0bab133R93

jrperritt · 2017-03-10T00:49:31Z

+}
+
+type HypervisorPage struct {
+	pagination.LinkedPageBase


If this is a linked page, it should have a way to get from one page to the next (usually a Links field). Does that exist?
I noticed there are Marker and Limit fields in the ListOpts, so maybe it's a MarkerPage?

@jrperritt You are right, it's actually a marker page. Sadly when I implemented it properly it seems our version of OpenStack doesn't support the marker field (e.g. needs Nova V2.33) and just spins forever. Do we have a standard way of catering for OpenStack's API stupidity regarding backwards compatibility?

I'm guessing lowest common denominator e.g. SinglePageBase

jrperritt · 2017-03-10T00:50:38Z

+	VCPUsUsed int `json:"vcpus_used"`
+}
+
+func (h *Hypervisor) UnmarshalJSON(b []byte) error {


Have a look at the other UnmarshalJSON functions for style changes in this method (variable name changes).

jrperritt · 2017-03-10T00:50:52Z

+	pagination.LinkedPageBase
+}
+
+func (page HypervisorPage) IsEmpty() (bool, error) {


Have a look at the other UnmarshalJSON functions for style changes in this method (variable name changes).

jrperritt · 2017-03-10T00:50:56Z

+	return len(hypervisors) == 0, err
+}
+
+func ExtractHypervisors(p pagination.Page) ([]Hypervisor, error) {


Have a look at the other UnmarshalJSON functions for style changes in this method (variable name changes).

For some reason this morning storage space started to (in gophercloud only not via curl) return sizes as scientific notation

Our environment doesn't support marker pages, so go for the backwards compatible option.

coveralls · 2017-03-27T10:48:26Z

Coverage increased (+0.1%) to 69.597% when pulling 4202358 on spjmurray:compute_hypervisors_list into f47ca3a on gophercloud:master.

jtopjian · 2017-05-11T18:33:09Z

+func (r *Hypervisor) UnmarshalJSON(b []byte) error {
+
+	type tmp Hypervisor
+	var s *struct {


@jrperritt I'm unable to find Go documentation to explain when *struct is required. All other UnmarshalJSON functions use struct. Testing with both seems to result in the same.

Is *struct required here?

To clarify a little bit:

I can do some separate / local tests with struct and *struct and see the differences there, but it seems here, json.Unmarshal will happily take both. Just not sure if *struct should be respected here.

Is *struct required here?

No, and we should maintain consistency with the other UnmarshalJSON methods [by changing it to a non-pointer].

jtopjian · 2017-05-18T16:18:44Z

I've cleaned this up a bit and going to resubmit this in a new PR. @spjmurray's changes will remain in the commit history.

…updates Networking v2: Updates to openstack_networking_router_v2

spjmurray mentioned this pull request Nov 3, 2016

Hypervisors Support #179

Closed

spjmurray force-pushed the compute_hypervisors_list branch from 10f6a0a to e4c6181 Compare November 3, 2016 15:44

spjmurray changed the title ~~Nova Hypervisors API~~ [WIP] Nova Hypervisors API Nov 3, 2016

spjmurray mentioned this pull request Nov 8, 2016

OpenStack Input Plugin influxdata/telegraf#2013

Closed

Nova Hypervisors API

530d34d

Adds in support to list all available hypervisors (detailed) for applications such as monitoring and capacity planning. For gophercloud#179

spjmurray force-pushed the compute_hypervisors_list branch from e4c6181 to 530d34d Compare February 20, 2017 11:48

spjmurray changed the title ~~[WIP] Nova Hypervisors API~~ Nova Hypervisors API Feb 20, 2017

jrperritt suggested changes Mar 10, 2017

View reviewed changes

spjmurray added 4 commits March 23, 2017 14:55

Fix more API weirdness

1e0bb47

For some reason this morning storage space started to (in gophercloud only not via curl) return sizes as scientific notation

Stylistic changes for jrperritt

5d5e2c0

Change to be a marker paged type

e0feaf7

Revert to single page results

4202358

Our environment doesn't support marker pages, so go for the backwards compatible option.

jtopjian reviewed May 11, 2017

View reviewed changes

jtopjian closed this May 18, 2017

jtopjian mentioned this pull request May 18, 2017

compute/v2: Hypervisors List API #344

Merged

cardoe pushed a commit to cardoe/gophercloud that referenced this pull request Aug 27, 2020

Merge pull request gophercloud#178 from jtopjian/networkingv2-router-…

0065316

…updates Networking v2: Updates to openstack_networking_router_v2

Conversation

spjmurray commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtopjian commented Nov 3, 2016

Uh oh!

spjmurray commented Nov 3, 2016

Uh oh!

jtopjian commented Nov 3, 2016

Uh oh!

spjmurray commented Nov 3, 2016

Uh oh!

coveralls commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrperritt commented Jan 12, 2017

Uh oh!

spjmurray commented Feb 20, 2017

Uh oh!

coveralls commented Feb 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrperritt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Mar 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtopjian commented May 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

spjmurray commented Nov 3, 2016 •

edited

Loading

coveralls commented Nov 3, 2016 •

edited

Loading

coveralls commented Nov 3, 2016 •

edited

Loading

coveralls commented Feb 20, 2017 •

edited

Loading

coveralls commented Mar 27, 2017 •

edited

Loading