openstack: move metadata fetch under daemon writer and reduce runs #248
Conversation
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
Results can be seen here: https://paste.opendev.org/show/bHK9oRLZlbSc4jfUjH0y/ |
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
2 similar comments
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
@atyronesmith sorry, last small change: https://github.com/k8snetworkplumbingwg/sriov-network-operator/compare/37c550226cbc26849293ffd1eb20713f620b7dd2..494763265806686a1e9d52b1f5d309b3313d978b |
|
/approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
oh sorry, I obviously have no review rights here! I hope I didn't mess with automation. |
pkg/utils/utils_virtual.go
Outdated
| return | ||
| } | ||
|
|
||
| func parseOpenstackMetaData(pciAddr string, metaData OSPMetaData, networkData OSPNetworkData) (networkID string, macAddress string) { |
There was a problem hiding this comment.
will the metaData.Devices not be nil if the ReadOpenstackMetaData function doesn't return anything or return error?
There was a problem hiding this comment.
I added a check:
if metaData.Devices == nil {
return "", ""
}Tell me what you think.
There was a problem hiding this comment.
maybe just better to use a pointer for the structs and leave the old validation?
if metaData == nil || networkData == nil {
return
}
There was a problem hiding this comment.
@SchSeba can you please look at this one again? Is it ok now? thanks
There was a problem hiding this comment.
let me test one more time, to make sure it works fine
There was a problem hiding this comment.
ok it works as expected: https://paste.opendev.org/show/bI7Tm3zNEWWtcvnoYh0t/
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
3 similar comments
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
/lgtm /cc @zshi-redhat @pliurh |
Is there a use case of hot plugging? |
OpenStack Nova doesn't support hotplugging SRIOV devices. This is in the roadmap but not implemented yet, and won't be before a bit of time I think. It's safe to assume that hotplug is not a thing for now. |
Machines are immutable, so there is no need to fetch for metadata every 30s. This patch will move out the functions to read the metadata file and do it in the daemon writer. Old behavior: Every 30 seconds, for each network device found, fetch, read and process OpenStack metadata. New behavior: daemon process does a first run then runs a routine that executes every 30 seconds. So we'll only do fetch metadata two times in total. Also, in case the API contract is broken, we might encounter some issues so let's switch to the latest stable, 2018-08-27, which has all the data that we need and is currently in the same format as latest for our needs.
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Machines are immutable, so there is no need to fetch for metadata every
30s.
This patch will move out the functions to read the metadata file and do
it in the daemon writer.
Old behavior:
Every 30 seconds, for each network device found, fetch, read and process
OpenStack metadata.
New behavior:
daemon process does a first run then runs a routine that executes every
30 seconds. So we'll only do fetch metadata two times in total.
Also in case the API contract is broken, we might encounter some issues so
let's switch to the latest stable, 2018-08-27, which has all the data
that we need and is currently in the same format as latest for our
needs.