Controller runtime 0.2.0 alpha0 vendor #1369
Conversation
openshift-ci-robot
added
size/XXL
|
Hi @corinnekrych. Thanks for your PR. I'm waiting for a operator-framework or openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
Hi @corinnekrych thanks so much for this PR! 🎉 We might not be merging it soon as that would be breaking changes and we need to decide in which release we would want to have this. The release of controller-runtime is also an alpha one, so we might need to wait for a more stable release of that. There also needs to be a migration doc for the changes, and some other things so maybe would make sense to have someone else from our team work on this with you if you are interested in this? Thanks again! cc @joelanford @shawn-hurley @estroz @hasbro17 @AlexNPavel @theishshah |
|
/ok-to-test |
openshift-ci-robot
added
ok-to-test
|
@corinnekrych: The following tests 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. DetailsInstructions 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. |
|
I'd rather wait for a |
| err = wait.PollImmediate(time.Second*1, time.Second*60, func() (bool, error) { | ||
| pods := &v1.PodList{} | ||
| err = runtimeClient.List(context.TODO(), &client.ListOptions{LabelSelector: set.AsSelector()}, pods) | ||
| err = runtimeClient.List(context.TODO(), pods, client.MatchingLabels(set)) |
There was a problem hiding this comment.
Hi @corinnekrych,
In the f494845#diff-b38c855ac3ed0aa95294c855ea839f99R176 it was solved as follows.
err = runtimeClient.List(context.TODO(), pods, client.MatchingLabels(dep.Spec.Selector.MatchLabels))| break | ||
| } | ||
| lo := client.InNamespace(r.Namespace) | ||
| lo := client.ListOptions{Namespace: r.Namespace} |
There was a problem hiding this comment.
here we can do just as follows.
lo := &client.ListOptions{}
lo.InNamespace(r.Namespace)| syncer := zapcore.AddSync(destWriter) | ||
| conf := getConfig() | ||
|
|
||
| conf.encoder = &logf.KubeAwareEncoder{Encoder: conf.encoder, Verbose: conf.level.Level() < 0} |
There was a problem hiding this comment.
Following it fixed made in the PR: f494845#diff-4407315726717f30063d558928ad9188R37
zapf "sigs.k8s.io/controller-runtime/pkg/log/zap"
conf.encoder = &zapf.KubeAwareEncoder{Encoder: conf.encoder, Verbose: conf.level.Level() < 0}
`
|
Hi @corinnekrych, This PR is not passing in the tests, we could address all that is done here in the PR #1388. Could we agree in close this one and move together with the PR #1388 which has these changes and is passing in all tests? c/c @lilic |
|
@lilic I'm fine with it. Go ahead and close. I'll try yours. |
|
HI @corinnekrych, I think that just the owners can close it. Could you please close this one? Let me know if you face some issue with #1388. Cheers, |
|
Closing in favour of #1388, thanks! |
5 participants
Description of the change:
Adjust
List(gCtx goctx.Context, list runtime.Object, opts ...dynclient.ListOptionFuncto match new runtime-controller signature.Motivation for the change:
Make operator-sdk compatible with runtime-controller