Kubernetes SIG Testing, 6 Mar 2023

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: SIG Testing Weekly Meeting for 20230306

Description

SIG Testing Weekly Meeting for 20230306

A

A

A

Hey bro, hey man, how are you doing.

B

Fine, how about you.

A

I'm good I can hear your cat in the background. You'll, probably.

B

C

A

Well, yeah she's very upset right now.

D

E

B

Instead of light yearly, all the time.

D

A

We'll give it a few more minutes, then we'll get started.

A

If you're new, which I don't know, if we have anybody new, uh you can add to the um recurring topics, and please add yourself to the attendance.

A

Thank you. Everyone um I think we can uh go ahead and get started. um Michelle you're, the only one who can share your screen, I think. So, if you want to share the screen, we can do that. Otherwise uh it doesn't look like we have that much um I, don't think we have anybody new, but good morning, everybody good afternoon, Wherever, You, Are um I, think we can go ahead and move on to the open discussion.

A

um Patrick has a few things on here, but he's not here right now, so um we can take a look at them and if anybody else has anything, please add to the list or speak up and and talk about it.

A

Please go for it.

D

C

Hey so my question is for Michelle.

A

Yeah, what's up.

C

uh I would like to know: what's the process to add a beer cluster to brow, an external one. I know the tooling part, which is basically use chain, create generate a cube config and had that as a secret, but from employer perspective, are you allowed to basically do that?

C

uh It's it's an amazing question, so you can you can follow up. We can follow up on slack if you want, like.

E

Yeah I I think this is one of the things where I probably need to poke folks uh more directly I'm proud, because I think they might have a better idea of what the policy is around that but um yeah I think follow-up after sounds great.

C

Okay, I might I might try to bring call or Linus I, don't know.

E

Yeah cool Alanis uh child's not directly on that anymore, but child may also know um but I think polar Alanis is uh are good. People.

C

E

C

Yeah, if you can't bring them before being demos like that would be great yeah.

E

I have uh stand up after this. Usually so I can go uh poke them. There.

C

Yeah, because we we got an eks cluster, you want to plug that in Cloud as soon as possible.

E

Yeah, if you have a, um if you have anything, to link as well feel free to uh send that to me and I can pass it on.

C

No wait, I think I have to reach you somewhere cool.

D

What's the plan to have mixed uh clusters for testing.

C

Yeah, so we want to expand the the footprint for the broadcaster.

D

C

More yeah, the idea is like in the beginning, move all the pre-submates running against the KK Ripple on eks and later and later, and that's only pre Summit that doesn't require it. We test basically.

D

Why do we want to make it look? The the experience that they have is assumed that you add something new. We are going to hit issues so I. Don't think that we should start with the presumency in KK. We should start with something that you know is less important.

C

What do you mean less important? Look.

D

If we have this new project and something happens, we are going to block all the developers in in kubernetes right.

C

Not necessarily because uh we are not going in production by the way, we're basically trying to establish what the other to identify. What are the blocker to move specific projects to AKs like how the expression, storage, exhaustions like and CPU memory, those kind of things we are not saying. Oh, we plug an eks cluster and we go in production right away.

C

The idea is to have we need to plug it, because we have just one control, plane and half Canary jobs on that broadcaster and we try to identify- and we also need to communicate to the community that we're doing that. So flakiness is expected. Yeah.

D

But I mean floating is expected, but we shouldn't do it with KK. We have tons of products that can serve as canary.

C

Yeah, but we only have gke.

C

We only have gke to support all the projects and if you move the rest of the project inside the Google infrastructure to us we're going to have a cost problem, because no.

D

No I'm not saying that. No, no, no, no generally I'm, not saying that we shouldn't have eks. What I'm saying is that we should have a rollout strategy right and and eqs the first candidate shouldn't be KK is what I say.

D

We should have uh no dependency, and ideally we should be able to practice this from nowhere. What I'm saying is, uh if we're going to practice faster, we can start with periodics or with presents in this side projects you know, and when we have some soft time we say well. This is ready for going for kubernetes, but as first thing adding to the KK I think that is very risk.

C

Yeah I think rollout strategy is not established because we didn't do a full inventory of all the projects we want to migrate. But ultimately we need to plug ATS and that's my first goal here. I first want to plug eks to the pro control plane and later I'm gonna reach out to pre-seek testing and Order C to to basically identify which Pro job we want to migrate. We don't have to migrate everything, but we need to establish a list of.

D

Jobs then we haven't seen yeah I thought that you wanted to add this new cluster 4K kks first thing. So that's why I was scared, because no.

C

We we need to be able to plug that eks cluster and try to have kind of reach up branding there from the control plane.

D

C

Doesn't have to be, presumpted doesn't have to be, but Summit only periodic I think we can just have periodic running there. No, no! No! Look! We.

D

Should add everything to say we should not have kubernetes kubernetes as the first thing, because then then is is very painful. I, don't know how many peers sorry for for not being cleared at the effort, so.

C

Okay, I see Patrick in the call, so I think that's it. For me,.

A

Yeah, thank you very much. Thank you guys and Patrick. We can Circle back around to you. um I did replace your second link because I think it was just a duplicate.

F

It is, it is, um and the first one actually got emerged. The second one was a fix for it and then the third one, the one that is currently pending is in kubernetes. Let me just get you the link, um it's very the subject is very very golden lint I can I can update the document later or because.

A

Of that one should just added.

F

Yeah I think that's for one, seven or two, but that looks right. Yep, that's one! So current status is that I think middle of last year, I proposed to strengthen our linting of pull requests.

F

uh It would be stronger than what we have enforced for existing codes, but that's that makes sense because new code might have a higher might might have to meet a higher quality bar, and we do have additional tools that we just can't apply to the current code base, merely because there are so many things in it that just weren't found when when adding the code, the current status is that we could merge this one pending kubernetes PR, and then there is a shell script, that users can run to check their own code, that they are currently writing and in a pull request.

F

There is a new job that explicitly needs to be started with a slash test command and then it will run additional tests against the new code in a PR.

F

So this this kubernetes PR that that currently needs a reviewer I think Antonio looked at it a while back um and this this is now ready to for merging The Next Step and that's where help would be needed would be to make the user experience nicer. So. Currently, what happens is say a developer or a reviewer remembers to start a job. Then there is a failed brow job.

F

The contributor needs to look into the text output of that job, to figure out which code lines need to be updated and that's all less convenient than in other repositories, where a GitHub action executes that and directory posts. The result to the patch few in GitHub. We can't do that in kubernetes or not easily, because we need to run this check in Pro.

F

Github action doesn't work that well because it wouldn't it would interact poorly with the merge robot and there is an issue open about supporting GitHub annotations from brow, but it hasn't made much progress. So if someone wants to do some useful work here, there was a volunteer who signed up for it kubecon Europe last year, but he just hasn't had the time. So this is for a test info issue. The last one 17056 is up for someone to work on that.

B

Might have changed but I believe originally the issue was you had to be a GitHub app to use that API and that's a more involved change to Pro.

F

um Okay, well, whatever the solution is I think I think the latest information, the latest design, was that we in Pro we just write some text files, those get archived as artifacts and then um some post-processing of the result, the same one. That posts commit comments on the pr would also look at these text files and update GitHub I. Think that was the latest status on on how we wanted to solve that, whether, if there's now a different solution that would be would be fine. Of course, too.

B

Yeah Patrick: are you referring to the check CPI.

B

Because if that's the case, that's um that's a rough starter issue. Migrating Pro to uh a GitHub app is gonna require no.

F

No, no on the running side at all, not at all I I was referring just to where we run go along Sea Island So. Currently, it runs as as part of uh pool kubernetes verify, and if we keep it like that, we would just produce additional artifacts from that verify run and those results somehow need to get into GitHub uh annotations.

F

So we it's currently fairly easy to write additional artifacts files. It's just that they will be sitting there in the artifacts folder of that failed brow, job and nothing will will happen with them. That's that's the missing part.

F

Well, I guess: unless we have a volunteer to work on that, it will just well we'll just keep that issue open. We can move forward. I think we could more move forward with this kubernetes pull request. I hope, that's just a matter of approving it now and then I I can send an email to the cabinet's death mailing list about announcing that this functionality is now available, can try it out and if it makes sense we can gradually um uh run this job more often say. Currently it's all it's opt-in.

F

We could make it so that it always runs. Then we could make it merge blocking, depending on on what we, what we experience is so.

B

One thing that we've done before is just fold Linson to verify and add a mechanism to permit certain failures, graduate incrementally and ratcheted.

B

Have we considered like making the default verify lint, run more linters and then filtering the output I think going see. Island puts like what winter failed, yeah.

F

Yes, so you you could do that the question so go along. Sea Island can suppress issues, but then the question is what how do people know that it failed if it doesn't cause a job failure? Well,.

B

So so what we've done in other places is we've captured the output in the Shell wrapper and then, if it's for files that we know should are not yet look they're either in an allow list or a deny list, we compute. If we want to fail or not so so like when we've started enforcing shell check, uh it was not possible to get it to pass on the whole repo.

B

um So we just had a list of like known shell check failure files and then, um if your file wasn't in the list of no not yet fixed files, then we failed and not by matching the output. You.

F

Could you could all do all of that with golang C Island the conflict? You can specify, suppress an error in certain files, but I don't think we want that I think what we want is.

B

Well, so we we still print the error. We just don't exit non-zero.

F

You could also do that with golang client in the config. You can change the severity of an error to informational.

B

You can do that per directory, because I think I think we might want to consider something like that. It's cheaper to run, go link, Sea Island once and people will actually look at the verify output and we don't- and we get by pretty well without uh bot comments with that um and then what we do is we ask people to work on cutting down the exclusion list? Okay and new code won't be in the excluded list.

F

But I'm not sure how that helps in the pull request. So suppose someone writes new code yeah, the new code. Does the new code doesn't pass for stricter linting tests? Yes, what what would be the experience in the pool gel and the pull requests would so like, like.

B

Verify script would pass right; no, it fails on new files is how we set it up before. So when we've incrementally wrapped.

F

Okay, okay, yeah! That's that doesn't really help because say if someone adds a new test to e to e-test to an existing file with the setup.

B

That I've, just there's, there's two things that happen: yeah, there's totally new files are not permitted to fail. The new linters existing files that are known to fail are permitted until they're removed from the exclusion list, and then we ask people to work on removing them from the exclusion list. Over time you don't have files on the exclusion list and then new code in those files also can't yeah.

F

So this the current mechanism is more intelligent than that. You can't add new chunks and you function to an existing file must pass valenter checks, okay and that works without configuration changes. It looks at the base base, hash and automatically figures out which code is new and only reports, issues in modified lines.

B

Will that is it smart enough to do that locally or am I only going to get those results if, on CI.

F

The script that I have is intelligent enough that you can do that yourself locally.

F

There is a very the the verify golang CI lint sh script has a minus a parameter for automatic, and that looks at your current branch and reports. The strict or you can, you can say minus a minus s and it will check your local branch with a strict configuration and that's the same thing. That would happen in the CI.

B

I'll just point out two things and then Michelle's had her hand up um yeah, I.

D

F

B

Exit strategy to to where we don't need to run uh both of these, and we just have standardized on new lints um I'm, also just with my Kate semper hat on I'm, a little I'm not excited to add another precipit that has to independently run over all of the code.

B

um We are trying to remove Chris emits because they're expensive. They run on every push, yeah um and I'm, not sure what we've allocated this one, but typically go. Link CI lint, just with our huge amount of code, has actually like we're running that and verify with a full like Ci node. um If, if.

F

We think that this new functionality is ready to be applied in a mandatory way for all pull requests. Then it can run as part of pool kubernetes verify.

F

The same way that I just described it would be a new verify script that runs golang, Sea Island, again with a different configuration, but it would be in the same pool, kubernetes verify okay and it would use for cached results. That's the other beauty of golang Clan. It caches the the previous analysis results, so the second one would be fairly.

B

Fast I think we want to do that. Another option you have is, um we can make it not default under the make verify command, but if we're not quite ready yet, but we can have CI run it there and we can encode it to be you can we can in the script logic we can make it uh like optional until we're ready to make it required.

F

B

uh Which we might just be ready to now, but I'm going to stop talking now, Lynn for sure.

E

Yeah no I, don't know the point. The only thing I was thinking in terms of reporting is that I know pro has the link command, which is obviously not the same as this but I'm wondering if there's like a way to reshape that, um or maybe add some additional functionality that would fit in here um like needs to be triggered manually, but the I think, because it is like a proud Community does report the results of that to a comment on the pr um yeah.

B

Slash lint is kind of a broken approach. The problem with Slash lint is, it actually runs in the web hook process. So for security reasons, then we're like not worried about like letting arbitrary code inside of a proud job comment back with the official prow account, which could be disastrous right.

B

um Actually, as is if you had arbitrary, if you had like new linters I could like sneak some code in a linter that made it do things, but the the real problem is it's using the Go version that was built to compile the web hook and not the Go version under test uh and all or like the link tools and things you really want to control the version of your linter in the repo. So you can like roll forward and fix changes.

B

The I think that's the main reason we don't see much adoption of it, it just it skews versus what you're actually trying to test and I um I, actually kind of think. Maybe we should remove that plug-in. I, don't think it's very effect effective and.

A

B

Expensive to it's one of the most expensive things that you can trigger in a comment.

E

Okay, yeah I was about to say, like.

B

In that case, it has to like clone the code and then it and then like run, go lint over it in process uh and um most projects are using going CI lint instead. Anyhow.

E

Yeah, so actually, if.

E

It's a link job that actually sounds preferable.

B

I mean the nice thing about the lint command. Is that it is that it annotates your uh PR right.

E

But it sounds like that's the main advantage. Yeah.

B

And it would, we can have like a secondary account or something do this um I think we should probably make that a like enhancement to linting that doesn't block uh uh enabling ratcheting the lint and we've done that we've done that before um it sounds like we have a couple of options for that cool I. Think we've actually had relatively good experience, even with the pretty dumb approach, where we're just like grapping against the output versus uh like permission list, it has the biggest problem we've run into.

B

There is, if you get a lot of people trying to fix it, then you have merge conflicts on the list.

B

F

Okay, so do we have a conclusion that we move forward with a separate job for now, give it a few I, don't know weeks or so and gather feedback and then decide whether we want to move it to Paul kubernetes verify.

B

This sounds reasonable enough to uh move in to verify as soon as we can confirm that it's working honestly.

F

um That's that's the feedback part yeah, but I I posted some examples. In that open, pull request and I have another job. Another pull request opened that intentionally will fail the the linting. So we can use those as test cases and we can wait for other people to try it out and give us feedback.

B

I guess the other question becomes so you're enabling a new set of lenders now against new code. um I haven't dug in how you can figure this. Will this work, if we say I want to end say going Clint adds another linter and I want to add that one do we need to do? We need a third target, or can we add it to this one and and that one like there's different sets of? Is it new code.

F

It there's one set of additional linters compared to existing code and that's in the config file. Golang CI, strict GMO I have current. The current set is the default linters in golang Sea, Island yeah, plus the one that we have an explicitly enabled for existing code. How.

B

How do we move things from being a totally new linter, after after we've rolled this out, eventually to being required for everything.

F

um A new lender would need to be added to that config file, and it would have to pass all new code immediately. I.

B

E

F

We can't otherwise we can't edit there. If we get into the situation that we want to roll out a new lender. We basically would need to First fix existing code, I suppose and then add it and then say we are confident enough that this Linder works, but we enforce it immediately. I, don't think there is any kind of credit uh where we can make it optional, not not without another make Target or a.

B

Job, okay, because the thing that I think I'm missing here is so like when we roll out Child Check, and we have to just exclusion list file by doing that. Eventually, we got to a state where all new code and all existing code always requires this to pass yeah yeah. It.

D

Sounds like we have a.

B

Way to require all net new changes past this, but not necessarily an easy path towards okay, we've been fixing code, and now it's gonna.

F

Be right here, I think I think there is I. Think the what I mentioned earlier about changing visibility of a check or of an issue should get around. That.

B

Yeah but who's going to change that because having the list of we know all these things are failing, then you just tell people like cut down the list and we get there um I'm not saying we need to do that, but I I'm not sure. What's the forcing function, the Sim like the equivalent forcing function here to stop permitting it in existing files.

F

With its per file and per issue, so you basically can have a wild card for say this directory tree needs to pass.

B

So, are we going to have people start adding to a list of these are known to be fixed.

F

We could do that in by editing the config okay.

B

I think it I I would so suggest. I think it may be easier to get people to cut down a list of exceptions, even if it's large initially uh because you're then you're driving it towards Zero versus like an ever expanding set of required files and because also then you have to convince people to to add that, whereas what we, the other thing, that we did that I'm recalling now, we could do the inverse of this.

B

But I think the the missing part that I forgot to mention is so in the linters, where we've had the exception file. We also if we discover that a file is now passing. We require you to remove it from the exception file and we tell approvers you shouldn't be permitting adding things to the exception file like it would have to be pretty it unusual circumstances to permit that.

B

So every time you file a PR, if you like, delete some code and now that- and that was the offending code, and now that file is passing the the like lint script- notices that that file no longer has any failures. But it's in the in the like known, failing list and you have to remove it from the known failing list. So new new failures can't be introduced.

F

Yeah I don't think we have. We don't have a mechanism like that: finding config files that are stale and need to be removed, I guess that would require some additional wrapping around golang Sea Island to find Divine bosses.

B

I, don't know that this needs to block, but I think you should consider the how we drive towards the state of like it's a default lint.

F

Okay, yeah I, guess the the the we ask for the community event would indeed be to run, restrict config against existing code and submitting PRS for certain regions to fix issues that they find. This way.

E

Basically, like a long-term thing that does check for exclusions in the uh exclusions of the stricter linters in the entire code base and then, like also in report, we.

F

Don't we don't even need the exclusionalist to be honest, I think it's enough that we have a way to force new code to not add things, and then we can have a periodic job that just posts known issues. People can look at that list or run it locally and react on that yeah and then over time.

F

As we update code or as people submit PRS to fix lint issues, we will come to a state where some things automatically will not have both issues anymore, and we just need to tell people that yeah. We are taking this serious and we are welcoming patches to to fix these issues, and then people will start working on it. I suppose.

B

I guess the exit State, then, is once people have since we're not permanent and and divs, and we would ask people to go make dips to fix it. At some point. We notice that if you ran the tool, there is no old code with it and we just add it to the default job, exactly yeah yeah. That sounds a little trickier to ensure happens, but if we write up an issue outlining how to do it, I bet we can get there and like do one for each of the linters.

E

This sounds like good, uh like initial contributions too, so hopefully that.

B

E

Be nice for folks.

F

My experience is that people jump on these kind of things. We just need to make sure that they do it properly, but some of these things are fairly easy. It's it's goal Center or some, but we'll Industries give you sometimes even the the exact code that that needs to be applied.

F

There's even a golf go along CI lint fix option that, unfortunately, it doesn't work for all issues or all kinds of of code changes, but yeah some some things are fairly low hanging fruits to fix. If we just agree that, yes, we we want those PRS.

F

The main problem that I've had in the past, with wanting people to do things, is that we had no mechanism to enforce it for new new code, so we have had some open issues against the E to e-testing where I said this, this is a bad pattern. Let's remove it and then people start fixing something and then new people not aware of this guideline added it in other tests. So it was another never-ending battle against updating codes.

F

With this proposal here we are making sure that the new rules get enforced and then gradually we can get enforced for new code automatically and we can gradually fix your existing code. Thank you.

B

I think we have a good um approach here. Let's, uh let's test your test drive, make sure it's working and then let's, um let's get this running and verify.

F

B

Thank you for working on this.

A

Yeah, thank you, Patrick, okay, uh I think Patrick. You may also still need a volunteer for one of those issues. If that is something that we still want to pursue, um but that looks like it's the end of our discussion. If anybody has anything else, please speak up or hold your peace and I'm gonna go eat. Lunch.

A

Well, that sounds good to me. Thank you. Everybody I hope you have a wonderful afternoon or night.

E

B

Care thanks all.