-
Notifications
You must be signed in to change notification settings - Fork 382
Bump OSB client lib #2689
Bump OSB client lib #2689
Conversation
/hold |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jberkhahn 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 |
7a60c96
to
f3b2f3f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not sure where the original test-namespace is coming from.
¯_(ツ)_/¯
/lgtm
ObjectMeta: metav1.ObjectMeta{ | ||
Name: namespace, | ||
Namespace: namespace, | ||
UID: "testnamespace1234", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UID is specifically what was missing from the current implementation.
if r == nil { | ||
return nil, UnexpectedActionError() | ||
} | ||
if req.ServiceID == "" || req.PlanID == "" || req.OrganizationGUID == "" || req.SpaceGUID == "" { | ||
return nil, RequiredFieldsMissingError() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're trying to avoid hitting this check.
test/integration/controller_test.go
Outdated
@@ -748,6 +748,7 @@ func newControllerTestTestController(ct *controllerTest) ( | |||
// create a fake kube client | |||
fakeKubeClient := &fake.Clientset{} | |||
fakeKubeClient.Lock() | |||
prependGetNamespaceReaction(fakeKubeClient, "test-namespace") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use const testNamespace
it's already defined https://github.com/kubernetes-sigs/service-catalog/blob/f3b2f3ff22a819b1f6bf3915d6ab32315433d94e/test/integration/controller_test.go#L59
name = "github.com/pmorie/go-open-service-broker-client" | ||
packages = [ | ||
"v2", | ||
"v2/fake", | ||
"v2/generator", | ||
] | ||
pruneopts = "NUT" | ||
revision = "6988c0983446576f2cefc90112028a66e6137233" | ||
revision = "a894d21a6d93b2b01b434b2226d806cc1240f79d" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should lock that revision in Gopkg.toml file,
[[constraint]]
name = "github.com/pmorie/go-open-service-broker-client"
revision = "a894d21a6d93b2b01b434b2226d806cc1240f79d"
thanks to that executing dep ensure -v
will not replace that automatically with new version in the future.
@MHBauer what u mean by saying "Still not sure where the original test-namespace is coming from."? |
/test pull-service-catalog-integration |
ObjectMeta: metav1.ObjectMeta{ | ||
Name: namespace, | ||
Namespace: namespace, | ||
UID: "testnamespace1234", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to do that also here:
https://github.com/kubernetes-sigs/service-catalog/blob/master/pkg/controller/case_test.go#L87-L91
sth like:
import "k8s.io/apimachinery/pkg/util/uuid"
k8sClient.CoreV1().Namespaces().Create(&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: testNamespace,
UID: uuid.NewUUID(), // fake clientset doesn't duplicate server-side behavior like uid assignment
},
})
f3b2f3f
to
8680980
Compare
pkg/controller/case_test.go
Outdated
@@ -25,6 +25,9 @@ import ( | |||
"testing" | |||
"time" | |||
|
|||
"sync" | |||
|
|||
"github.com/google/uuid" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"github.com/google/uuid" | |
"k8s.io/apimachinery/pkg/util/uuid" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woops
8680980
to
8884038
Compare
/test pull-build-all-images-for-ppc64le |
- lock dependency version to a894d21a6d93b2b01b434b2226d806cc1240f79d
8884038
to
1e3f56e
Compare
/test pull-build-all-images-for-ppc64le |
1 similar comment
/test pull-build-all-images-for-ppc64le |
/lgtm |
@jberkhahn: The following test 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. |
Also lock version of the osb client lib