OpenStack: Use correct type to unmarshal clouds #3000
Conversation
openshift-ci-robot
added
the
size/S
Fedosin
force-pushed
the
otherclouds
branch
2 times, most recently
from
54a3a38
to
59ae7a8
Compare
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
Now, to unmarshal clouds from yaml files we always use type Clouds https://github.com/gophercloud/utils/blob/master/openstack/clientconfig/results.go#L10-L15 But this is wrong because for clouds-public.yaml we should use PublicClouds https://github.com/gophercloud/utils/blob/master/openstack/clientconfig/results.go#L3-L8 otherwise LoadPublicCloudsYAML function always returns an empty map. Fixes: openshift#2997
openshift-ci-robot
added
size/M
Fedosin
changed the title
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
@fabianofranz PTAL :) |
|
/retest |
1 similar comment
|
/retest |
| } | ||
|
|
||
| func loadAndLog(fn func() (string, []byte, error)) (map[string]clientconfig.Cloud, error) { | ||
| func loadAndLog(fn func() (string, []byte, error)) ([]byte, error) { |
There was a problem hiding this comment.
Why are we changing the return type to []byte? All LoadXCloudsYaml() functions return map[string]Cloud
There was a problem hiding this comment.
Now in loadAndLog function we always unmarshal data to clientconfig.Clouds type, but this is wrong, because for clouds-public.yaml we must use clientconfig.PublicClouds, otherwise the result will be empty and the installation fail.
To avoid this I decided to keep only loading of content and logging in loadAndLog, and move the unmarshaling to the upper level.
In LoadCloudsYAML and LoadSecureCloudsYAML we continue to unmarshal to clientconfig.Clouds, but for LoadPublicCloudsYAML we now use clientconfig.PublicClouds
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Fedosin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
@Fedosin: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Now, to unmarshal clouds from yaml files we always use type
Cloudshttps://github.com/gophercloud/utils/blob/master/openstack/clientconfig/results.go#L10-L15
But this is wrong because for clouds-public.yaml we should use
PublicCloudshttps://github.com/gophercloud/utils/blob/master/openstack/clientconfig/results.go#L3-L8
otherwise
LoadPublicCloudsYAMLfunction always returns an empty map.Fixes: #2997