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

Bump dependencies #1 #700

Merged
merged 1 commit into from
Oct 30, 2021
Merged

Bump dependencies #1 #700

merged 1 commit into from
Oct 30, 2021

Conversation

kuritka
Copy link
Collaborator

@kuritka kuritka commented Oct 26, 2021

related to #697

  • k8s.io/* bundle to v0.22.2 (controller-gen tool makes changes in CRD description after bump)
  • github.com/golang/mock to v1.6.0 (is not compiled into final binary but checked by dependabot)
  • github.com/miekg/dns to v1.1.43
  • github.com/infobloxopen/infoblox-go-client to v1.1.1 (changes argument CreateTXTRecord from integer to uint)
  • sigs.k8s.io/controller-runtime to v0.10.2, had to fix the test, see comment below.
  • sigs.k8s.io/external-dns is problematic in terms it uses latest github.com/go-logr/logr which is incompatible
    with previous versions. Will be part of followup PR

controller-runtime explained

Controller-runtime works on copies of annotations rather than their pointers, so I had to modify the test.

func TestGslbProperlyPropagatesAnnotationDownToIngress(t *testing.T) {
        // arrange
        settings := provideSettings(t, predefinedConfig)
        expectedAnnotations := map[string]string{"annotation": "test"}
        settings.gslb.Annotations = expectedAnnotations
        err := settings.client.Update(context.TODO(), settings.gslb)
        require.NoError(t, err, "Can't update gslb")
        // act
        reconcileAndUpdateGslb(t, settings)
        err2 := settings.client.Get(context.TODO(), settings.request.NamespacedName, settings.ingress)
        // assert
        assert.NoError(t, err2, "Failed to get expected ingress")
        assert.Equal(t, expectedAnnotations, settings.ingress.Annotations)
}

If I extend fial assertion the passing test is doing this:

assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.ingress.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.gslb.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, expectedAnnotations)

The "k8gb.io/strategy": "roundRobin" is added to expectedAnnotations during reconcileAndUpdateGslb and controlle-runtime
is the guy which extends expectedAnnotations.

I bumped controller runtime to v0.10.2, and only this concrete test has to be updated, because expectedAnnotations are not altered within controller-runtime:

assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.ingress.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.gslb.Annotations)
assert.Equal(t, map[string]string{"annotation": "test"}, expectedAnnotations)

Signed-off-by: kuritka kuritka@gmail.com

k0da
k0da previously approved these changes Oct 26, 2021
kuritka added a commit that referenced this pull request Oct 27, 2021
Approve #700 first, this is follow-up.

Controller-runtime works on copies of annotations rather than their pointers, so I had to modify the test.

```go
func TestGslbProperlyPropagatesAnnotationDownToIngress(t *testing.T) {
	// arrange
	settings := provideSettings(t, predefinedConfig)
	expectedAnnotations := map[string]string{"annotation": "test"}
	settings.gslb.Annotations = expectedAnnotations
	err := settings.client.Update(context.TODO(), settings.gslb)
	require.NoError(t, err, "Can't update gslb")
	// act
	reconcileAndUpdateGslb(t, settings)
	err2 := settings.client.Get(context.TODO(), settings.request.NamespacedName, settings.ingress)
	// assert
	assert.NoError(t, err2, "Failed to get expected ingress")
	assert.Equal(t, expectedAnnotations, settings.ingress.Annotations)
}
```

If I extend fial assertion the passing test is doing this:
```go
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.ingress.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.gslb.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, expectedAnnotations)
```

The `"k8gb.io/strategy": "roundRobin"` is added to `expectedAnnotations` during `reconcileAndUpdateGslb` and controlle-runtime
is the guy which extends `expectedAnnotations`.

I bumped controller runtime to `v0.10.2`, and only this concrete test has to be updated, because expectedAnnotations are not altered within controller-runtime:
```go
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.ingress.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.gslb.Annotations)
assert.Equal(t, map[string]string{"annotation": "test"}, expectedAnnotations)
```

Signed-off-by: kuritka <kuritka@gmail.com>
kuritka added a commit that referenced this pull request Oct 27, 2021
Approve #700 first, this is follow-up.

Controller-runtime works on copies of annotations rather than their pointers, so I had to modify the test.

```go
func TestGslbProperlyPropagatesAnnotationDownToIngress(t *testing.T) {
	// arrange
	settings := provideSettings(t, predefinedConfig)
	expectedAnnotations := map[string]string{"annotation": "test"}
	settings.gslb.Annotations = expectedAnnotations
	err := settings.client.Update(context.TODO(), settings.gslb)
	require.NoError(t, err, "Can't update gslb")
	// act
	reconcileAndUpdateGslb(t, settings)
	err2 := settings.client.Get(context.TODO(), settings.request.NamespacedName, settings.ingress)
	// assert
	assert.NoError(t, err2, "Failed to get expected ingress")
	assert.Equal(t, expectedAnnotations, settings.ingress.Annotations)
}
```

If I extend fial assertion the passing test is doing this:
```go
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.ingress.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.gslb.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, expectedAnnotations)
```

The `"k8gb.io/strategy": "roundRobin"` is added to `expectedAnnotations` during `reconcileAndUpdateGslb` and controlle-runtime
is the guy which extends `expectedAnnotations`.

I bumped controller runtime to `v0.10.2`, and only this concrete test has to be updated, because expectedAnnotations are not altered within controller-runtime:
```go
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.ingress.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.gslb.Annotations)
assert.Equal(t, map[string]string{"annotation": "test"}, expectedAnnotations)
```

Signed-off-by: kuritka <kuritka@gmail.com>
related to #697

 `k8s.io/*` bundle to `v0.22.2` (controller-gen tool makes changes in CRD description after bump)
 - `github.com/golang/mock` to `v1.6.0` (is not compiled into final binary but checked by dependabot)
 - `github.com/miekg/dns` to `v1.1.43`
 - `github.com/infobloxopen/infoblox-go-client` to `v1.1.1` (changes argument CreateTXTRecord from integer to uint)
 - `sigs.k8s.io/controller-runtime` to `v0.10.2`, had to fix the test, see comment below.
 - `sigs.k8s.io/external-dns` is problematic in terms it uses latest `github.com/go-logr/logr` which is incompatible
with previous versions. Will be part of followup PR

Controller-runtime works on copies of annotations rather than their pointers, so I had to modify the test.

```go
func TestGslbProperlyPropagatesAnnotationDownToIngress(t *testing.T) {
        // arrange
        settings := provideSettings(t, predefinedConfig)
        expectedAnnotations := map[string]string{"annotation": "test"}
        settings.gslb.Annotations = expectedAnnotations
        err := settings.client.Update(context.TODO(), settings.gslb)
        require.NoError(t, err, "Can't update gslb")
        // act
        reconcileAndUpdateGslb(t, settings)
        err2 := settings.client.Get(context.TODO(), settings.request.NamespacedName, settings.ingress)
        // assert
        assert.NoError(t, err2, "Failed to get expected ingress")
        assert.Equal(t, expectedAnnotations, settings.ingress.Annotations)
}
```

If I extend fial assertion the passing test is doing this:
```go
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.ingress.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.gslb.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, expectedAnnotations)
```

The `"k8gb.io/strategy": "roundRobin"` is added to `expectedAnnotations` during `reconcileAndUpdateGslb` and controlle-runtime
is the guy which extends `expectedAnnotations`.

I bumped controller runtime to `v0.10.2`, and only this concrete test has to be updated, because expectedAnnotations are not altered within controller-runtime:
```go
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.ingress.Annotations)
assert.Equal(t, map[string]string{"annotation": "test", "k8gb.io/strategy": "roundRobin"}, settings.gslb.Annotations)
assert.Equal(t, map[string]string{"annotation": "test"}, expectedAnnotations)
```

Signed-off-by: kuritka <kuritka@gmail.com>
@kuritka
Copy link
Collaborator Author

kuritka commented Oct 27, 2021

I extended by controller-runtime v0.10.2

Paths must begin with a '/'. When unspecified,
all paths from incoming requests are matched.
Paths must begin with a '/' and must be present
when using PathType with value "Exact" or "Prefix".
Copy link
Member

Choose a reason for hiding this comment

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

Ist result of CRD regeneration?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ytsarev, yes, controller-gen makes changes in description after k8s.io/* bump.

@kuritka kuritka merged commit 607a9db into master Oct 30, 2021
@kuritka kuritka deleted the gosum3 branch October 30, 2021 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants