LOG-3792: Update operator dependencies for kubernetes #2095

jcantrill · 2023-07-17T17:54:05Z

Description

This PR updates dependencies for:

kubernetes apis
openshift apis
operator-sdk
go.mod go version
controllers to support preferred watch syntax
replaces deprecated functions and types

Links

https://issues.redhat.com/browse/LOG-3797

openshift-ci-robot · 2023-07-17T17:54:38Z

@jcantrill: This pull request references LOG-3792 which is a valid jira issue.

In response to this:

Description

This PR updates dependencies for:

kubernetes apis

openshift apis

operator-sdk

go.mod go version

controllers to support preferred watch syntax

Links

https://issues.redhat.com/browse/LOG-3797

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.

openshift-ci · 2023-07-17T17:57:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcantrill

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:

~~OWNERS~~ [jcantrill]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2023-07-21T19:18:51Z

@jcantrill: This pull request references LOG-3792 which is a valid jira issue.

In response to this:

Description

This PR updates dependencies for:

kubernetes apis

openshift apis

operator-sdk

go.mod go version

controllers to support preferred watch syntax

replaces deprecated functions and types

Links

https://issues.redhat.com/browse/LOG-3797

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.

jcantrill · 2023-07-25T13:50:19Z

/test functional

Clee2691 · 2023-07-25T18:53:35Z

/lgtm

openshift-ci-robot · 2023-07-25T20:00:33Z

/retest-required

Remaining retests: 0 against base HEAD 123524c and 2 for PR HEAD 41c1732 in total

openshift-ci-robot · 2023-07-25T22:00:18Z

/retest-required

Remaining retests: 0 against base HEAD e418bc8 and 1 for PR HEAD 41c1732 in total

jcantrill · 2023-07-27T17:18:42Z

/retest

jcantrill · 2023-07-27T17:32:49Z

/retest

jcantrill · 2023-07-27T18:03:16Z

/test functional

jcantrill · 2023-07-27T18:03:31Z

/test

openshift-ci · 2023-07-27T18:03:33Z

@jcantrill: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

/test ci-index-cluster-logging-operator-bundle
/test e2e-target
/test functional-target
/test images
/test lint
/test unit

The following commands are available to trigger optional jobs:

/test e2e-ocp-target-minus-one
/test e2e-ocp-target-minus-two
/test functional

Use /test all to run all jobs.

In response to this:

/test

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.

jcantrill · 2023-07-27T18:03:49Z

/test all

jcantrill · 2023-07-27T19:29:29Z

/test functional-target

jcantrill · 2023-07-28T11:14:47Z

/test functional-target

syedriko · 2023-07-28T18:33:07Z

controllers/clusterlogging/clusterlogging_controller.go

+				DeleteFunc: func(evt event.DeleteEvent) bool {
+					return IsElasticsearchRelatedObj(evt.Object)
+				},
+				GenericFunc: func(evt event.GenericEvent) bool {


We can drop the evt here

syedriko · 2023-07-28T18:47:29Z

controllers/clusterlogging/clusterlogging_controller.go

+		Owns(&corev1.ServiceAccount{}).
+		Owns(&appsv1.Deployment{}).
+		Owns(&appsv1.DaemonSet{}).
+		Owns(&corev1.Secret{},


I think we need to keep the old Watches() stanza for the Elastic Secrets or else we end up with

Owns(&corev1.Secret{}). ... Owns(&corev1.Secret{}, builder.WithPredicates(...

where the first Owns() subsumes all the events of the second.

CL doesn't own the Elastic Secrets and the old Watches() caters to that.

@jcantrill I think this is important to address...

internal/collector/trust_bundle.go

internal/utils/sets/types.go

syedriko · 2023-07-29T03:21:03Z

internal/utils/utils.go

@@ -263,15 +265,15 @@ func WriteToWorkingDirFile(toFile string, value []byte) error {
 }

 func init() {
-	rand.Seed(time.Now().UnixNano())
+	rgen = rand.New(rand.NewSource(time.Now().UnixNano()))


we actively do not want this: "this source is not safe for concurrent use"
https://pkg.go.dev/math/rand#NewSource

Moved into test/helpers given we no longer use this (was used for Logstore) but I do not think it matters. We are not relying on any consistently generated "Random" name

internal/utils/utils.go

syedriko · 2023-07-29T03:30:07Z

internal/utils/utils.go

 }

 var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789")

 func GetRandomWord(wordSize int) []byte {
 	b := make([]rune, wordSize)
 	for i := range b {
-		b[i] = letters[rand.Intn(len(letters))] //nolint:gosec
+		b[i] = letters[rgen.Intn(len(letters))] //nolint:gosec


Since Go 1.20, we can just call rand.Intn(). There's no longer a need to manually seed the generator: https://cs.opensource.google/go/go/+/refs/tags/go1.20.6:src/math/rand/rand.go;l=306-313

test/framework/functional/output_cloudwatch.go

syedriko · 2023-07-29T03:41:15Z

test/framework/functional/output_cloudwatch.go

@@ -70,7 +70,7 @@ func (f *CollectorFunctionalFramework) GetLogStreamsByGroup(client *cwl.Client,
 		myStreams        []string
 		logStreamsOutput *cwl.DescribeLogStreamsOutput
 	)
-	err := wait.PollImmediate(defaultRetryInterval, f.GetMaxReadDuration(), func() (done bool, err error) {
+	err := wait.PollUntilContextTimeout(context.TODO(), defaultRetryInterval, f.GetMaxReadDuration(), true, func(cxt context.Context) (done bool, err error) {
 		// TODO: need to query for log group or get more log group info above
 		logStreamsOutput, err = client.DescribeLogStreams(
 			context.TODO(),


context.TODO() -> cxt

test/framework/functional/output_cloudwatch.go

test/helpers/loki/receiver.go

syedriko · 2023-07-29T04:00:34Z

test/helpers/loki/receiver.go

@@ -179,8 +180,7 @@ func (r *Receiver) Query(logQL string, orgID string, limit int) ([]StreamValues,
 // QueryUntil repeats the query until at least n lines are received.
 func (r *Receiver) QueryUntil(logQL string, orgID string, n int) (values []StreamValues, err error) {


Might want to add context.Context to the Query() method and pass it on to the http request like above. For direct calls to Query(), call Query(context.TODO()...), there're only three places its called from.

I find 13. This change is primary meant to update dependencies and remove dead code. I'd like to limit the scope as much as possible

syedriko

Please see inline

syedriko · 2023-08-03T21:08:24Z

.bingo/junitreport.mod

@@ -2,7 +2,4 @@ module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT

 go 1.14


Does this need an update to 1.20?

All our tests pass so doesn't look critical

syedriko · 2023-08-03T22:01:20Z

controllers/clusterlogging/clusterlogging_controller.go

+		Owns(&corev1.ServiceAccount{}).
+		Owns(&appsv1.Deployment{}).
+		Owns(&appsv1.DaemonSet{}).
+		Watches(&source.Kind{Type: &corev1.Secret{}},


I think we still need an Owns(&rbacv1.Secret{}) so we receive events for the Secrets that CL owns.

jcantrill · 2023-08-04T14:43:22Z

/test functional-target

openshift-ci · 2023-08-04T15:36:04Z

@jcantrill: all tests passed!

Full PR test history. Your PR dashboard.

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.

syedriko · 2023-08-04T19:32:24Z

/lgtm

jcantrill added the release/5.8 label Jul 17, 2023

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 17, 2023

openshift-ci bot requested review from Clee2691 and vimalk78 July 17, 2023 17:56

openshift-ci bot added the midstream/Dockerfile A Dockerfile.in sync is needed with midstream label Jul 17, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 17, 2023

jcantrill force-pushed the log3797 branch 2 times, most recently from a92a3ab to 545627f Compare July 21, 2023 19:18

jcantrill force-pushed the log3797 branch from 545627f to 41c1732 Compare July 24, 2023 18:22

openshift-ci bot assigned Clee2691 Jul 25, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 25, 2023

jcantrill force-pushed the log3797 branch from 41c1732 to bc62d51 Compare July 26, 2023 19:30

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 26, 2023

jcantrill force-pushed the log3797 branch 2 times, most recently from 0ec9114 to ca5b8ba Compare July 27, 2023 14:55

syedriko reviewed Jul 28, 2023

View reviewed changes