Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bumped dependencies #1420

Merged
merged 12 commits into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/crds/kudo.dev_instances.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: instances.kudo.dev
spec:
Expand Down
2 changes: 1 addition & 1 deletion config/crds/kudo.dev_operators.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operators.kudo.dev
spec:
Expand Down
2 changes: 1 addition & 1 deletion config/crds/kudo.dev_operatorversions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operatorversions.kudo.dev
spec:
Expand Down
38 changes: 11 additions & 27 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,63 +10,47 @@ require (
github.com/containerd/containerd v1.2.9 // indirect
github.com/docker/docker v1.4.2-0.20190916154449-92cc603036dd
github.com/docker/go-connections v0.4.0 // indirect
github.com/docker/go-units v0.4.0 // indirect
github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0
github.com/emicklei/go-restful v2.9.6+incompatible // indirect
github.com/ghodss/yaml v1.0.0 // indirect
github.com/go-bindata/go-bindata v3.1.2+incompatible
github.com/go-openapi/jsonreference v0.19.3 // indirect
github.com/go-openapi/spec v0.19.3 // indirect
github.com/go-openapi/validate v0.19.2
github.com/gogo/protobuf v1.3.1 // indirect
github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect
github.com/google/btree v1.0.0 // indirect
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
github.com/gophercloud/gophercloud v0.2.0 // indirect
github.com/gorilla/context v1.1.1 // indirect
github.com/gorilla/mux v1.6.2 // indirect
github.com/gosuri/uitable v0.0.4
github.com/huandu/xstrings v1.2.0 // indirect
github.com/imdario/mergo v0.3.7 // indirect
github.com/json-iterator/go v1.1.8 // indirect
github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect
github.com/mailru/easyjson v0.7.0 // indirect
github.com/manifoldco/promptui v0.6.0
github.com/mattn/go-colorable v0.1.4 // indirect
github.com/mattn/go-runewidth v0.0.4 // indirect
github.com/mitchellh/copystructure v1.0.0 // indirect
github.com/morikuni/aec v1.0.0 // indirect
github.com/onsi/ginkgo v1.10.1 // indirect
github.com/onsi/gomega v1.7.1 // indirect
github.com/opencontainers/image-spec v1.0.1 // indirect
github.com/pmezard/go-difflib v1.0.0
github.com/prometheus/client_golang v1.0.0 // indirect
github.com/spf13/afero v1.2.2
github.com/spf13/cobra v0.0.5
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.4.0
github.com/thoas/go-funk v0.5.0
github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca
go.uber.org/atomic v1.4.0 // indirect
go.uber.org/zap v1.10.0 // indirect
golang.org/x/crypto v0.0.0-20190923035154-9ee001bba392 // indirect
golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
golang.org/x/tools v0.0.0-20191025023517-2077df36852e // indirect
gopkg.in/yaml.v2 v2.2.7
gopkg.in/yaml.v2 v2.2.8
gotest.tools v2.2.0+incompatible
k8s.io/api v0.16.6
k8s.io/apiextensions-apiserver v0.0.0-20190918161926-8f644eb6e783
k8s.io/apimachinery v0.16.6
k8s.io/client-go v0.16.6
k8s.io/code-generator v0.16.6
k8s.io/component-base v0.16.6
k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a // indirect
k8s.io/kubectl v0.16.6
sigs.k8s.io/controller-runtime v0.4.0
sigs.k8s.io/controller-tools v0.2.4
k8s.io/api v0.17.3
k8s.io/apiextensions-apiserver v0.17.2
k8s.io/apimachinery v0.17.3
k8s.io/client-go v0.17.3
k8s.io/code-generator v0.17.3
k8s.io/component-base v0.17.3
k8s.io/kubectl v0.17.3
sigs.k8s.io/controller-runtime v0.5.1
sigs.k8s.io/controller-tools v0.2.6
sigs.k8s.io/kind v0.6.1
sigs.k8s.io/yaml v1.1.0
)

replace k8s.io/code-generator v0.16.6 => github.com/kudobuilder/code-generator v0.16.6-beta.0.0.20200221140535-0ef46f1228ff
replace k8s.io/code-generator v0.17.3 => github.com/kudobuilder/code-generator v0.17.3
zen-dog marked this conversation as resolved.
Show resolved Hide resolved
183 changes: 107 additions & 76 deletions go.sum

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/engine/task/task_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func patchResource(modifiedObj, currentObj runtime.Object, ctx Context) error {
}

// Execute the patchResource
err = ctx.Client.Patch(context.TODO(), modifiedObj, client.ConstantPatch(patchType, patchData))
err = ctx.Client.Patch(context.TODO(), modifiedObj, client.RawPatch(patchType, patchData))
if err != nil {
return fmt.Errorf("failed to execute patch: %v", err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/kudoctl/cmd/testdata/deploy-kudo-ns.yaml.golden
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operators.kudo.dev
spec:
Expand Down Expand Up @@ -76,7 +76,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operatorversions.kudo.dev
spec:
Expand Down Expand Up @@ -398,7 +398,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: instances.kudo.dev
spec:
Expand Down
6 changes: 3 additions & 3 deletions pkg/kudoctl/cmd/testdata/deploy-kudo-sa.yaml.golden
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operators.kudo.dev
spec:
Expand Down Expand Up @@ -76,7 +76,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operatorversions.kudo.dev
spec:
Expand Down Expand Up @@ -398,7 +398,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: instances.kudo.dev
spec:
Expand Down
6 changes: 3 additions & 3 deletions pkg/kudoctl/cmd/testdata/deploy-kudo-webhook.yaml.golden
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operators.kudo.dev
spec:
Expand Down Expand Up @@ -76,7 +76,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operatorversions.kudo.dev
spec:
Expand Down Expand Up @@ -398,7 +398,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: instances.kudo.dev
spec:
Expand Down
6 changes: 3 additions & 3 deletions pkg/kudoctl/cmd/testdata/deploy-kudo.yaml.golden
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operators.kudo.dev
spec:
Expand Down Expand Up @@ -76,7 +76,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: operatorversions.kudo.dev
spec:
Expand Down Expand Up @@ -398,7 +398,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.2.4
controller-gen.kubebuilder.io/version: v0.2.6
creationTimestamp: null
name: instances.kudo.dev
spec:
Expand Down
6 changes: 3 additions & 3 deletions pkg/kudoctl/kudoinit/crd/bindata.go

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions pkg/kudoctl/util/kudo/kudo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,17 @@ func TestKudoClient_OperatorExistsInCluster(t *testing.T) {
}

tests := []struct {
bool bool
exists bool
zen-dog marked this conversation as resolved.
Show resolved Hide resolved
err string
createns string
getns string
obj *v1beta1.Operator
}{
{}, // 1
{createns: "default", getns: "default"}, // 2
{bool: true, obj: &obj}, // 3
{bool: true, createns: "default", obj: &obj}, // 4
{getns: "kudo", obj: &obj}, // 4
{exists: true, obj: &obj}, // 3
{exists: true, createns: "default", obj: &obj, getns: "default"}, // 4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was failing without explicitly using namespace default.

{getns: "kudo", obj: &obj}, // 5
}

for i, tt := range tests {
Expand All @@ -72,8 +72,8 @@ func TestKudoClient_OperatorExistsInCluster(t *testing.T) {
// test if Operator exists in namespace
exist := k2o.OperatorExistsInCluster("test", tt.getns)

if tt.bool != exist {
t.Errorf("%d:\nexpected: %v\n got: %v", i+1, tt.bool, exist)
if tt.exists != exist {
t.Errorf("%d:\nexpected: %v\n got: %v", i+1, tt.exists, exist)
}
}
}
Expand Down
27 changes: 16 additions & 11 deletions pkg/test/step_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/discovery"
"k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

Expand All @@ -32,7 +33,7 @@ func TestStepClean(t *testing.T) {
pod2WithNamespace := testutils.NewPod("hello2", testNamespace)
pod2WithDiffNamespace := testutils.NewPod("hello2", "different-namespace")

cl := fake.NewFakeClient(pod, pod2WithNamespace, pod2WithDiffNamespace)
cl := fake.NewFakeClientWithScheme(scheme.Scheme, pod, pod2WithNamespace, pod2WithDiffNamespace)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewFakeClient method is deprecated


step := Step{
Apply: []runtime.Object{
Expand All @@ -53,16 +54,16 @@ func TestStepClean(t *testing.T) {
// Each test provides a path to a set of test steps and their rendered result.
func TestStepCreate(t *testing.T) {

pod := testutils.NewPod("hello", "")
pod := testutils.NewPod("hello", "default")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another case where a test started to fail without an explicit default namespace.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I feel like this behavior as tested is wrong. It should only be setting the namespace if it isn't set. Do you know what changed?

Copy link
Contributor Author

@zen-dog zen-dog Mar 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, for anybody who is interested: after some researching and debugging, I believe it was this change in go-client testing fixture that made this test fail. The commit description says it all.
As for the default client, this is also explicitly forbidden. Another mystery solved.

podWithNamespace := testutils.NewPod("hello2", "different-namespace")
clusterScopedResource := testutils.NewResource("v1", "Namespace", "my-namespace", "")
podToUpdate := testutils.NewPod("update-me", "")
clusterScopedResource := testutils.NewResource("v1", "Namespace", "my-namespace", "default")
podToUpdate := testutils.NewPod("update-me", "default")
specToApply := map[string]interface{}{
"replicas": int64(2),
}
updateToApply := testutils.WithSpec(t, podToUpdate, specToApply)

cl := fake.NewFakeClient(testutils.WithNamespace(podToUpdate, testNamespace))
cl := fake.NewFakeClientWithScheme(scheme.Scheme, testutils.WithNamespace(podToUpdate, testNamespace))

step := Step{
Logger: testutils.NewTestLogger(t, ""),
Expand Down Expand Up @@ -94,7 +95,7 @@ func TestStepDeleteExisting(t *testing.T) {
podToDeleteDefaultNS := testutils.NewPod("also-delete-me", "default")
podToKeep := testutils.NewPod("keep-me", testNamespace)

cl := fake.NewFakeClient(podToDelete, podToKeep, podToDeleteDefaultNS)
cl := fake.NewFakeClientWithScheme(scheme.Scheme, podToDelete, podToKeep, podToDeleteDefaultNS)

step := Step{
Logger: testutils.NewTestLogger(t, ""),
Expand Down Expand Up @@ -177,8 +178,10 @@ func TestCheckResource(t *testing.T) {
assert.Nil(t, err)

step := Step{
Logger: testutils.NewTestLogger(t, ""),
Client: func(bool) (client.Client, error) { return fake.NewFakeClient(test.actual), nil },
Logger: testutils.NewTestLogger(t, ""),
Client: func(bool) (client.Client, error) {
return fake.NewFakeClientWithScheme(scheme.Scheme, test.actual), nil
},
DiscoveryClient: func() (discovery.DiscoveryInterface, error) { return fakeDiscovery, nil },
}

Expand Down Expand Up @@ -226,8 +229,10 @@ func TestCheckResourceAbsent(t *testing.T) {
assert.Nil(t, err)

step := Step{
Logger: testutils.NewTestLogger(t, ""),
Client: func(bool) (client.Client, error) { return fake.NewFakeClient(test.actual), nil },
Logger: testutils.NewTestLogger(t, ""),
Client: func(bool) (client.Client, error) {
return fake.NewFakeClientWithScheme(scheme.Scheme, test.actual), nil
},
DiscoveryClient: func() (discovery.DiscoveryInterface, error) { return fakeDiscovery, nil },
}

Expand Down Expand Up @@ -296,7 +301,7 @@ func TestRun(t *testing.T) {
Timeout: 1,
}

cl := fake.NewFakeClient()
cl := fake.NewFakeClientWithScheme(scheme.Scheme)

test.Step.Client = func(bool) (client.Client, error) { return cl, nil }
test.Step.DiscoveryClient = func() (discovery.DiscoveryInterface, error) { return testutils.FakeDiscoveryClient(), nil }
Expand Down
5 changes: 0 additions & 5 deletions pkg/test/test_data/crd-in-step/00-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,3 @@ status:
singular: mycrd
storedVersions:
- v1beta1
conditions:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did something in the CRD API change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have one more condition now:

  - type: KubernetesAPIApprovalPolicyConformant
    reason: ApprovedAnnotation
    status: "True"

and with that, the test became flaky. I'm not sure if it takes a while for the transition to happen or smth. else

Copy link
Contributor Author

@zen-dog zen-dog Mar 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so I've got to the bottom of it (which in the hindsight wasn't very deep): with three conditions, they sometimes simply come out of order and test-harness expects the same order of elements when comparing slices in the IsSubset method. A quick test confirms:

assert.Nil(t, IsSubset(
	[]string{"foo", "bar"},
	[]string{"bar", "foo"},
))

fails with: 
Expected nil, but got: &utils.SubsetError{path:[]string(nil), message:"value mismatch, expected: foo != actual: bar"}

However, it turns out we can not omit the conditions here:

  • the first step creates the CRD and assumes that they're created
  • the second step creates a CR of that CRD

but the second create-step (not the assert one) occasionally fails with the server could not find the requested resource because the CRD hasn't gone through all transitions yet.

I'd suggest fixing IsSubset comparing slices but I'm not sure about all the implications. /cc @justinbarrick

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, right now it is order dependent, I need to evaluate existing tests to see if order is often important. IIRC @alenkacz and I discussed this a while back but I don't remember where we landed on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so I avoided the problem for this bump by changing the CRD name/group to prowjobs.prow.example.com (instead of *.k8s.io) which doesn't require the annotation and so we only have the same two conditions as before (these two do not come out of order).

However, the bigger question stays: I'm not sure if k8s guarantee array elements order in all places. We might want to relax the current subset comparison to be order independent or give the user the choice to use both (some sort of annotation maybe?)

- type: NamesAccepted
status: "True"
- type: Established
status: "True"
2 changes: 2 additions & 0 deletions pkg/test/test_data/crd-in-step/00-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: mycrds.mycrd.k8s.io
annotations:
api-approved.kubernetes.io: "https://github.com/kubernetes/enhancements/pull/1111"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the PR in the annotation for more details on why this is needed

spec:
group: mycrd.k8s.io
version: v1beta1
Expand Down
8 changes: 0 additions & 8 deletions pkg/test/test_data/create-or-update/00-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,5 @@ metadata:
status:
acceptedNames:
kind: ProwJob
# InitialNamesAccepted will change from False -> True once the CRD is ready.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

conditions:
- reason: NoConflicts
status: "True"
type: NamesAccepted
- reason: InitialNamesAccepted
status: "True"
type: Established
storedVersions:
- v1
2 changes: 2 additions & 0 deletions pkg/test/test_data/create-or-update/00-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: prowjobs.prow.k8s.io
annotations:
api-approved.kubernetes.io: "https://github.com/kubernetes/enhancements/pull/1111"
spec:
group: prow.k8s.io
version: v1
Expand Down
Loading