use availability_zone instead of availability (update godep for gophercloud) #44082

zetaab · 2017-04-05T07:01:08Z

What this PR does / why we need it: there is typo in json variable name

Which issue this PR fixes: fixes #44032

Special notes for your reviewer:our openstack environment region name is not nova, so I tested this and it works now

All cinder blockstorages are using variable name availability_zone instead of availability. Docs:

v3:
https://developer.openstack.org/api-ref/block-storage/v3/index.html?expanded=create-a-volume-detail#create-a-volume

v2:
https://developer.openstack.org/api-ref/block-storage/v2/index.html?expanded=create-volume-detail#create-volume

I could not find v1 documentation anymore from openstack pages. However, https://developer.rackspace.com/docs/cloud-block-storage/v1/api-reference/cbs-volumes-operations/#create-a-volume documentation says also availability_zone is the correct one.

Like mentioned in #44032 (comment) openstack CLI is using availability_zone

k8s-reviewable · 2017-04-05T07:01:13Z

This change is

k8s-ci-robot · 2017-04-05T07:01:15Z

Hi @zetaab. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

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.

cblecker · 2017-04-06T01:36:16Z

Hi @zetaab,
We can't directly modify a vendor, as they're versioned.

It looks like this was added to gophercloud here: gophercloud/gophercloud@b06120d

We could update the godep to pull this change in. Information on updating godeps is here: https://github.com/kubernetes/community/blob/master/contributors/devel/godep.md

zetaab · 2017-04-06T13:22:48Z

is that correctly updated deps? @cblecker I updated whole gophercloud/gophercloud

cblecker · 2017-04-06T15:12:59Z

You actually need to use the Godep tool to do this kind of update, as it tracks when the updates were done and the hashes of those updates.

Directions and details here: https://github.com/kubernetes/community/blob/master/contributors/devel/godep.md

zetaab · 2017-04-06T16:31:06Z

I tried to follow those instructions but it does not work, maybe someone who have done this before can do it?

 kubernetes [fixzone2●] % godep restore
godep: Dep (k8s.io/metrics/pkg/apis/custom_metrics) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/api/resource) not found
godep: Dep (k8s.io/metrics/pkg/apis/custom_metrics/install) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/apimachinery/announced) not found
godep: Dep (k8s.io/metrics/pkg/apis/custom_metrics/v1alpha1) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/api/resource) not found
godep: Dep (k8s.io/metrics/pkg/apis/metrics) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/apis/meta/v1) not found
godep: Dep (k8s.io/metrics/pkg/apis/metrics/install) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/apimachinery/announced) not found
godep: Dep (k8s.io/metrics/pkg/apis/metrics/v1alpha1) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/api/resource) not found
godep: Dep (k8s.io/metrics/pkg/client/clientset_generated/clientset) restored, but was unable to load it with error:
	Package (k8s.io/client-go/discovery) not found
godep: Dep (k8s.io/metrics/pkg/client/clientset_generated/clientset/fake) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/runtime) not found
godep: Dep (k8s.io/metrics/pkg/client/clientset_generated/clientset/scheme) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/apis/meta/v1) not found
godep: Dep (k8s.io/metrics/pkg/client/clientset_generated/clientset/typed/metrics/v1alpha1) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/apis/meta/v1) not found
godep: Dep (k8s.io/metrics/pkg/client/clientset_generated/clientset/typed/metrics/v1alpha1/fake) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/apis/meta/v1) not found
godep: Dep (k8s.io/metrics/pkg/client/custom_metrics) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/api/meta) not found
godep: Dep (k8s.io/metrics/pkg/client/custom_metrics/fake) restored, but was unable to load it with error:
	Package (k8s.io/apimachinery/pkg/labels) not found
godep: Error checking some deps.

kubernetes [fixzone2●●] % ./hack/godep-save.sh
godep: Package (google.golang.org/genproto/googleapis/bigtable/admin/v2) not found
!!! Error in ./hack/godep-save.sh:37
  Error in ./hack/godep-save.sh:37. 'GOPATH=${GOPATH}:${KUBE_ROOT}/staging godep save "${REQUIRED_BINS[@]}"' exited with status 1
Call stack:
  1: ./hack/godep-save.sh:37 main(...)
Exiting with status 1
 kubernetes [fixzone2●●] %

zetaab · 2017-04-07T08:35:33Z

@cblecker any ideas?

cblecker · 2017-04-07T20:14:03Z

Oh hmm.. try using hack/godep-restore.sh to restore the deps instead of invoking godep restore directly. I'll review and update those docs as they don't include that.

cblecker · 2017-04-07T21:22:31Z

I just tried this locally, and it seems to have worked. If you wouldn't mind testing on your environment to see if that allows you to update the godep, that would be great.

zetaab · 2017-04-08T06:28:39Z

@cblecker your instructions works pretty well, if those files are now OK :)

The only thing which did not work is verify godeps

% ./hack/verify-godeps.sh
Checking for 'Godeps/' changes against '/master'
fatal: Not a valid object name /master

i tried also execute godep update and rerun it, but it did not help

cblecker · 2017-04-08T06:42:16Z

Ah! Are you working off a git branch, or just a clone of master?

cblecker · 2017-04-08T06:46:08Z

@k8s-bot ok to test

zetaab · 2017-04-08T07:24:15Z

I was working in that fixzone2 branch

git checkout master
git merge fixzone2
./hack/verify-godeps.sh
Checking for 'Godeps/' changes against '/master'
fatal: Not a valid object name /master

I have no idea why those tests fail :)
edit: ah i need modify some code as well

zetaab · 2017-04-08T12:23:39Z

@cblecker thanks for helping 👍

grodrigues3 · 2017-04-13T18:20:15Z

@k8s-bot ok to test

xsgordon · 2017-04-14T23:37:33Z

/cc @k8s-sig-openstack-bugs

@idvoretskyi Can you add the sig/openstack label please?

dchen1107 · 2017-04-14T23:56:47Z

/lgtm

k8s-github-robot · 2017-04-14T23:57:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, pospispa, zetaab

Needs approval from an approver in each of these OWNERS Files:

~~OWNERS~~ [dchen1107]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

cblecker · 2017-04-17T18:17:59Z

@dchen1107 @zetaab Does this need a release note? Only piece that is blocking merge here.

zetaab · 2017-04-17T21:22:36Z

well, imo someone (who have access) could add release-note-none or release-note and remove release-note-label-needed and do-not-merge? This is basically godep update for gophercloud, and if i check previous changelog items, some of these are there. So if we just add release-note its fine for me(its added to changelog with that i think?).

cblecker · 2017-04-17T21:47:47Z

Release notes are needed when there is a breaking change, or a notable change in functionality (for better or worse). In this case, I this looks like just a bug fix.
/release-note-none

Will still need @dchen1107 to remove the do-not-merge

k8s-ci-robot · 2017-04-17T21:47:48Z

@cblecker: you can only set release notes if you are the author or an assignee.

In response to this:

Release notes are needed when there is a breaking change, or a notable change in functionality (for better or worse). In this case, I this looks like just a bug fix.
/release-note-none

Will still need @dchen1107 to remove the do-not-merge

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.

cblecker · 2017-04-17T21:47:56Z

/assign

cblecker · 2017-04-17T21:48:03Z

/release-note-none

zetaab · 2017-04-18T04:34:08Z

@k8s-bot non-cri e2e test this

pospispa · 2017-04-18T09:02:29Z

@k8s-bot non-cri e2e test this

k8s-github-robot · 2017-04-19T10:26:25Z

Automatic merge from submit-queue

idvoretskyi · 2017-04-19T15:14:17Z

cc @kubernetes/sig-openstack-bugs

This is "backport" of Kubernetes PR #44082 (kubernetes/kubernetes#44082). Actually, the github.com/gophercloud/gophercloud library version is bumped up to a version that contains the availability zone fix. The problem is described in Bugzilla #1444828 (https://bugzilla.redhat.com/show_bug.cgi?id=1444828). The problem was introduced into Kubernetes 1.6 and fixed in Kubernetes 1.7. That's why the fix is "backported" into OpenShift 3.6.

…0898813696af0 This is first part of the "backport" of Kubernetes PR #44082 (kubernetes/kubernetes#44082). Actually, the github.com/gophercloud/gophercloud library version is bumped up to a version that contains the availability zone fix. The problem is described in Bugzilla #1444828 (https://bugzilla.redhat.com/show_bug.cgi?id=1444828). The problem was introduced into Kubernetes 1.6 and fixed in Kubernetes 1.7. That's why the fix is "backported" into OpenShift 3.6.

This is second and last part of the "backport" of Kubernetes PR #44082 (kubernetes/kubernetes#44082). Actually, the github.com/gophercloud/gophercloud library version is bumped up to a version that contains the availability zone fix. The problem is described in Bugzilla #1444828 (https://bugzilla.redhat.com/show_bug.cgi?id=1444828). The problem was introduced into Kubernetes 1.6 and fixed in Kubernetes 1.7. That's why the fix is "backported" into OpenShift 3.6.

Automatic merge from submit-queue use availability_zone instead of availability (update godep for gophercloud) **What this PR does / why we need it**: there is typo in json variable name **Which issue this PR fixes**: fixes kubernetes#44032 **Special notes for your reviewer**:our openstack environment region name is not nova, so I tested this and it works now All cinder blockstorages are using variable name availability_zone instead of availability. Docs: v3: https://developer.openstack.org/api-ref/block-storage/v3/index.html?expanded=create-a-volume-detail#create-a-volume v2: https://developer.openstack.org/api-ref/block-storage/v2/index.html?expanded=create-volume-detail#create-volume I could not find v1 documentation anymore from openstack pages. However, https://developer.rackspace.com/docs/cloud-block-storage/v1/api-reference/cbs-volumes-operations/#create-a-volume documentation says also availability_zone is the correct one. Like mentioned in kubernetes#44032 (comment) openstack CLI is using availability_zone

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 5, 2017

k8s-github-robot assigned dchen1107 Apr 5, 2017

k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Apr 5, 2017

zetaab changed the title ~~use availability_zone instead of availability~~ use availability_zone instead of availability in gophercloud blockstorage API Apr 5, 2017

grodrigues3 added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 5, 2017

zetaab closed this Apr 6, 2017

zetaab reopened this Apr 6, 2017

k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 6, 2017

zetaab closed this Apr 8, 2017

update deps

5c24fb9

zetaab reopened this Apr 8, 2017

zetaab added 3 commits April 8, 2017 10:51

use AvailabilityZone instead of Availability

2fb9fc4

fix format

5aad93a

update staging godeps

3066783

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 13, 2017

dchen1107 added the area/provider/openstack Issues or PRs related to openstack provider label Apr 14, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 14, 2017

zetaab changed the title ~~use availability_zone instead of availability in gophercloud blockstorage API~~ use availability_zone instead of availability (update godep for gophercloud) Apr 17, 2017

k8s-ci-robot assigned cblecker Apr 17, 2017

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Apr 17, 2017

jsafrane removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Apr 19, 2017

k8s-github-robot merged commit 712ccf3 into kubernetes:master Apr 19, 2017

pospispa mentioned this pull request Apr 28, 2017

Backport: Availability Zone is Ignored by Cinder API openshift/origin#13954

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use availability_zone instead of availability (update godep for gophercloud) #44082

use availability_zone instead of availability (update godep for gophercloud) #44082

zetaab commented Apr 5, 2017 •

edited

k8s-reviewable commented Apr 5, 2017

k8s-ci-robot commented Apr 5, 2017

cblecker commented Apr 6, 2017

zetaab commented Apr 6, 2017 •

edited

cblecker commented Apr 6, 2017

zetaab commented Apr 6, 2017

zetaab commented Apr 7, 2017

cblecker commented Apr 7, 2017

cblecker commented Apr 7, 2017

zetaab commented Apr 8, 2017

cblecker commented Apr 8, 2017

cblecker commented Apr 8, 2017

zetaab commented Apr 8, 2017 •

edited

zetaab commented Apr 8, 2017

grodrigues3 commented Apr 13, 2017

xsgordon commented Apr 14, 2017

dchen1107 commented Apr 14, 2017

k8s-github-robot commented Apr 14, 2017

cblecker commented Apr 17, 2017

zetaab commented Apr 17, 2017

cblecker commented Apr 17, 2017

k8s-ci-robot commented Apr 17, 2017

cblecker commented Apr 17, 2017

cblecker commented Apr 17, 2017

zetaab commented Apr 18, 2017

pospispa commented Apr 18, 2017

k8s-github-robot commented Apr 19, 2017

idvoretskyi commented Apr 19, 2017

use availability_zone instead of availability (update godep for gophercloud) #44082

use availability_zone instead of availability (update godep for gophercloud) #44082

Conversation

zetaab commented Apr 5, 2017 • edited

k8s-reviewable commented Apr 5, 2017

k8s-ci-robot commented Apr 5, 2017

cblecker commented Apr 6, 2017

zetaab commented Apr 6, 2017 • edited

cblecker commented Apr 6, 2017

zetaab commented Apr 6, 2017

zetaab commented Apr 7, 2017

cblecker commented Apr 7, 2017

cblecker commented Apr 7, 2017

zetaab commented Apr 8, 2017

cblecker commented Apr 8, 2017

cblecker commented Apr 8, 2017

zetaab commented Apr 8, 2017 • edited

zetaab commented Apr 8, 2017

grodrigues3 commented Apr 13, 2017

xsgordon commented Apr 14, 2017

dchen1107 commented Apr 14, 2017

k8s-github-robot commented Apr 14, 2017

cblecker commented Apr 17, 2017

zetaab commented Apr 17, 2017

cblecker commented Apr 17, 2017

k8s-ci-robot commented Apr 17, 2017

cblecker commented Apr 17, 2017

cblecker commented Apr 17, 2017

zetaab commented Apr 18, 2017

pospispa commented Apr 18, 2017

k8s-github-robot commented Apr 19, 2017

idvoretskyi commented Apr 19, 2017

zetaab commented Apr 5, 2017 •

edited

zetaab commented Apr 6, 2017 •

edited

zetaab commented Apr 8, 2017 •

edited