Kubernetes KubeBuilder Controller Runtime and Controller Tools, 25 Mar 2021

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Kubernetes KubeBuilder Meeting 20210325

Description

KubeBuilder Meeting for 2021/03/25. See https://sigs.k8s.io/kubebuilder for more details.

A

Hello and welcome to the q builder controller, runtime and controller tools meeting for thursday march 25th 2021.

A

As a reminder, this is being recorded, so don't say anything you don't want recorded for all posterity and upload it to youtube. All right uh so looks like we have a few things on the agenda today. um I believe the first is a proposal for filtered list watches. So um if the author is there, uh do you want to take us away.

B

Hi, hello, yes, I'm here, like I don't know, I share the screen with the pr with the design document. I can talk about it. I don't know what how to follow.

A

Yeah, if you just want to um talk about it, um maybe give us a brief summary for people who haven't, read it and then um maybe we'll. We can do some discussion if there is sufficient quantity of people who have read it and have opinions.

B

Okay, there so.

B

So I'm I'm a member of kubernetes and we are using controller runtime in a tool that is deployed as diamond sets and we reconcile uh node resources.

B

B

In a big clusters, we found that the cpu and the memory on masters and was increasing like a lot, so we discovered that that control runtime was transferring the all the nodes to all the workers like every few minutes, because the node resource changed a lot because some annotations get added there because, for example, in cuber every few minutes we annotate the node with some information.

B

So it's like say a very lively resource, not the nodes, and so we discovered that we need to reconcile uh only the node where the pod was running on so for us. uh If we were able to filter the at least watch or select the list watch just for the node. The pod was running on was good enough for us.

B

So we, after an idea from alvaro aleman, we hack the cache and and add a selector to the list watch for the field name and it totally uh remove our our problems. So then we decide. Okay, let's try to see if we can put something upstream controller runtime because looks like there is an issue open there with something similar. Maybe it's not exactly that, but something similar.

B

So I end up creating creating a pr with a prototype and a design document that is quite simple. What it does is: yes, it it traverses the cache implementation, with some cash options and those cash options.

B

There are a pair of selection, selectors per resource, a field selector and label selector and at the end, what it does is just they just end up uh on the list watch as part of the list options, and that's all and of course it has some drawbacks like uh if you do so, the client is going to return uh 404, you try to s to get something that is not. uh That is that it has been filtered out by the selector so but.

B

But we have keep it like. You have to overwrite the cache, even in the ip of the design document, you have to overwrite the cache in case. You want this functionality like saying you know why, what you're doing it's not like a clean option in the builder or a clean option in the manager option? It's something like kind of hide hide builder inside the cat's package to override the cache, so users that use that they will know that they are doing this and they have to know the the consequences.

B

So that's it more or less.

A

Okay uh has has anybody else taken a look at this proposal? I was looking at it before the meeting a bit.

C

Yeah, so I've been reviewing it a bit and provided a bit of input so to provide some historic background on the question on why we never had anything for the filtering.

C

It's basically that we couldn't get to the point of finding a solution where we are happy with how the client copes with this, um because, as mentioned, if you uh change the cache, the client would be able to give you back anything. That's that gets filtered out, which means that this is a bit of a foot gun um this.

C

The kind of middle ground this concrete proposal would provide is that it adds the functionality just to the cache and then allows people to override the new cache func in the manager which is already possible today, and if you do that, it just said you're essentially saying you know better than the default, um so this would both give a possibility to do this I'll bite a bit hidden and not increase the foot should shoot potential so yeah. I would really like to see this.

B

Well, I have, I was entertaining an idea and I don't know if it's quite crazy, like I don't know, it makes sense to create even a wrapper that in case is not found in the cast or something it use directly the how control content call it api, reader, client or something like that.

B

So you are not going, you are going to find it, but you will have to pay the price of direct access to the masters or something I don't know if it makes any sense, I think, go ahead.

A

Oh, I was gonna say I think. That's probably I I think that would be very surprising um to me to get kind of an inconsistent usage pattern.

A

um I think I think this as it stands is probably uh like obvious enough quote unquote that something weird is going on that it's probably at the very least, a good first step. um Is that what were you gonna say over.

C

Yeah something along the similar lines that it's surprising, that it might cause issue with weight, limiting and so on. So I would prefer not to fall back to directly asking the api server.

A

I think the main comment I'd have on the on the design document is, um I would say, like, maybe just uh if you could drop a little section at the end there about like just uh like why why we're not doing the like, why we're not doing something more complicated or whatever? Just like a little bit of background of, we had a long debate here like this is kind of the minimal case, um and and this doesn't raise the questions of like how does the client interact or if we move it off onto the.

A

If we move it off onto a per controller basis, then we have to deal with like you know how do multiple controllers with different filters work so by just doing it here, we kind of sidestep all of that and it's a little bit less it's a little bit less.

A

You know ergonomic or magical, but it it sidesteps a lot of the long-running questions um and we could maybe we could always tackle those later if you want to, but like yeah, I think just having a section of like hey, let's we're we're sidestepping all these problems for now with this, and that's why this is the shape it is, um would be useful.

B

Okay, I will add.

A

Does anybody have anything else that they'd like to um add, or are we good to move on.

D

E

One quick, quick use case question that just occurred to me: does this I this is not necessarily the intent of this.

E

Would this feature open the door theoretically to like controller sharding like there's, you know been a handful of use cases of controllers that are trying to reconcile a tremendous number of resources, um or especially as we get into like multiple clusters. You know watching multiple clusters that kind of stuff.

E

I wonder if this opens a door to like okay now I could have two controllers things side by side, and each of them are like filtering out half of the world kind of stuff. Is that at all a scene uh use case or is that that possible through this? Do you think.

B

Well, as I see the the cache is per manager, so you cannot specify per controller.

E

Oh sure, oh sorry, yeah yeah, sorry that that is what I meant. I was using the word controller less than the control runtime sense and more in the like of kubernetes pod, sensible yeah. We could have like two pods running the same container. That's uh that's one of these managers.

A

I think, theoretically, you could do that yeah. um It would be neat to have a more comprehensive design for for sharding at some point, um but I think that might be a ways off, but yeah. I think. Theoretically, you could use this um as a building.

E

Block yeah, I would probably not suggest uh yeah trying to complicate the immediate proposal with that, but interesting to keep in mind.

A

Yeah definitely.

A

uh All right um alvaro, I think you have the next item.

C

Yes, uh I was wondering about maybe adding a variation of our client that, on top what you already have has the watch functionality.

C

um The background for that is explicitly not controller, since controllers are supposed to just use the cache backed client but clies.

C

This came up because we have a cli that is used a lot and does a lot, and this ended up causing lot issues, so we got asked to not pull but watch in the cli. This is something our client doesn't support at all and isn't needed for controllers either.

C

I ended up implementing this downstream, which works just fine, but it has to duplicate a bunch of private unexported code from our client, so yeah. I was thinking what about maybe adding a client with watch interface that has this on top of your or what we already have, but not changing the existing interface and keeping the existing interface everywhere. We already have it like the manager and so on.

A

Yeah, I think I think that probably seems reasonable uh uh just I. I would make it very clear in the docs that, like hey, uh this is this is like you probably don't need need this in normal use, but I think that that seems reasonable uh yeah, especially for the cli use cases, um and it can be useful like when writing little debugging scripts and stuff um as well yeah. I find myself using controller runtimes client for like little test cases and trying to figure out.

A

What's going on with an api server a lot so um yeah. I think it would be useful there too.

C

A

A

uh All right, uh the next one looks like it's about um our metrics support.

F

um Yes hi, this is suhani. um I work with an operator that brings in controller runtime and this operator works with clusters at scale. So recently we observed that one metric, it's called rest client, latency, duration, metric.

F

It ate up a significant portion of prometheus. I do have reservations about the cardinality of this metric, but for now I would I was wondering if we can find a way to disable this metric.

A

A

A

Oh it's! uh If it's deprecated and turned off uh that is probably worth probably worth us doing, that ourselves.

F

Sorry, uh the problem is that I looked into it. um It was deprecated and then turned off because it was renamed.

A

Oh okay, uh is it in upstream did I know like a while ago upstream, made a bunch of changes to cardinality to kind of try to fix some of this kind of stuff? Is this one of the metrics that has changes, and correspondingly do we need to fill around with our own definition?

A

That's, I guess that's kind of my long-term question the short-term question for um turning it off. um Are you looking for like a change in control or runtime or just like?

A

I is there, I'm not I'm not sure off the top of my head. If there's a good way in prometheus, to like, say, don't export this metric on a per controller basis or whatever does anyone know that off the top of their heads.

C

I think you can add a rule to drop certain metrics, but well I don't think you should ever have this metric, because the cardinality is just so high. It includes the object name.

C

So, in my opinion, the question is more: if we need to replace it with something or if we can just turn it off entirely, not if we can keep it.

A

Oh yeah yeah, that makes sense I kind of meant in the client. I wonder if there was anything client-side like if you have a registry to not export certain things, just as a quick patch that might actually be useful to have as a note in our documentation if that's possible, um but yeah.

A

I think I I'm curious to hear so what it I'm curious to see. What upstream does, I think, would be the the follow-up question I'd have, um I think, the that's it's a reasonable assertion that this is potentially very, very high cardinality and we shouldn't have it. um I know upstream tackled some of that recently, so I would be curious to hear what they did, because they found a way of reducing.

A

They ended up reducing a lot of the cardinality of some of these things. um So.

G

I think in our case I work with suhani. In our case, it was primarily caused by the fact that the uh the server that you're talking to is included as one of the labels and and uh we talked to a lot of servers and that that was where it fell over for us. um I would also mention: oh go ahead. Sorry.

A

No sorry that makes sense. I I.

G

We also have uh we implemented a metric like this ourselves, not knowing that this existed, and so so it is possible to do this yourself if we wanted to make the case that maybe it shouldn't be in controller run time, and you can do it yourself if you need to it's, not easy, it was a little complicated, but it was.

A

Possible yeah, um so I I feel like I feel like I don't have enough information off the top of my head to just say like let's remove it or let's not remove it, um but I am I'm definitely sympathetic to the issue. uh I'd say: maybe if we can follow up on the issue after the meeting with.

A

um I guess either either figuring out a good way to disable certain metrics um or or or something like that. uh I don't. I don't want to drop this metric, just like drop it in a release, because I think that would kind of suck uh for people who are relying on it so um seeing if we can figure out a way to let you filter out certain metrics um or if there already is a mechanism client side and if not maybe figuring out a deprecation plan and a short-term plan would be a good idea.

A

Does that seem reasonable to y'all.

F

Yeah, it seems pretty helpful um sorry I was looking at upstream as well. It doesn't look like they've made much changes as it pertains to this metric, but let's talk more and find a solution to this. Thank you.

A

A

um Okay, uh I think the next few are from material.

A

uh Is is it dirio here.

B

A

Think so yeah looks like not.

H

He's not to be.

A

Okay, we can we'll move these to the next one. um Yes all right, uh so the next one is uh generate, embedded object, meta in the crds.

A

All right yeah go ahead.

I

Hi, my name is daniel. I work on a couple of operators, mainly mini operator, which is object, storage and this pr introduces the capacity to generate embedded object method right now. It's actually skipped because it causes some some problems with the cubicity with the coordinates api, but there's some fields that are actually valuable uh to keep in place, especially because, if you, uh for example, our operator is, has a polygon claim template right and to reuse that we just said. We just declare: okay, we're gonna reuse.

I

This quantum claim template from staple sets, but when the the generated crd does not include the object method it this allows us to, for example, have fields such as annotations and labels, which are available to push down, because some csi drivers rely on those annotations to decide what to do with the volumes, and that's mainly the use case that we that we saw that okay, we definitely need to have on the generated crv, annotations uh and labels right. We were actually patching our generated crd, and then we thought, okay.

I

Why don't we improve the uh generate crd tool to actually keep it in place? But we understand that there's some fields that cause problems. uh I I mentioned them on the pr uh right. So so definitely I I skipped some of the fields that are uh problematic and only only kept the fields that make sense to keep on the object meta uh to be able to push them down all right. So this pr is mostly doing that right.

I

So uh as a lot of caution there, it used to be a specific case in the code in the cia degeneration case that it was specifically ignoring object, meta right, it was just keeping all the fields on object matter. It was probably someone already found that some of those fields caused trouble. So uh for this pr I already kind of like tested- and I saw there- was already plenty of conversations right on the issues.

I

I also listed those on the uh on the pr and I took all the feedback, and I think you you sold were trying to say someone said up here, but then you said: can you put a feature flag to turn it on and off? So I didn't put it like that, uh so that that's what speed this pr is about.

A

So this is, this is about um intermediate object matter right, like not the top level one exactly.

I

Yes, this is for when the crd has some intermediate uh object method in its description in in our case, in our operator, for example, uh it's the volume claim template that we need the object meta to still be sealed right, because um that metadata gives the the pvc some extra attributes, such as uh labels and annotations, mostly.

A

Yeah, no, I mean that um that makes sense. Let me.

A

A

um Okay, yeah, we did, we did never. We keep. We've been going back and forth for a while on this.

I

I

Yeah, so I built this this appear in late january, because we we were like patching the generated crd and I think there are some other users kind of like gassing. Can we have the object better remains because via customize, you could pretty much also patch it right. But then it's just some extra steps and I think it's there's some useful, so even uh even as a feature flag right right now it's introduced as disabled, so no one will notice a change unless they explicitly ask for it.

I

Passing the generate, embedded, object, meta flag, and now they will, they could actually make it so their crds. If they have the the need for the embedded meta, uh they could actually be generated from controller. So if the, if the flag which by default, is off it's it's not even present on dimension, no behavior will change. No one will notice. This has been uh added.

A

Yeah that that makes sense, uh I'm I'm long term- I'm not sure how I feel about a hard-coded list of fields, but I also acknowledge that I'm not entirely certain that there is a better option.

A

Because yeah, uh the full list of fields uh causes a ginormous.

I

Expansion, yes, uh that I agree uh and that's separate problem that the the pr on the right side will mention or the next year I mean, but uh yes, it does increase uh a little bit the size of the crd and there's kind of like a hard limit on the size of the crd to like 256k.

I

I think um so. So that's why it's optional right! It's like an opt-in kind of thing like. If you really need it, then you, you should know how much your own generation is going to actually be growing. But then uh again the whole point is that only people who need it is actually gonna come looking for this flag and it's like okay, I noticed my object method is missing. Oh there's a flag to make sure it gets generated, that's pretty much! It.

A

All right yeah, uh I think I think this seems fine honestly, we might. We might want to turn this on by default, um but yeah. Otherwise, I think this seems fine. uh I the only the only immediate comment, I would say is um actually I need to double check to see. If it's there, I thought I didn't see it yeah. I would.

A

I would maybe link to the comment from stefan in the code comments um stuff on sdts um in the comment by the list of fields that we allow, just so that in case someone comes upon it like a year in the future and they're like why this specific list of fields we can say well, someone from api machinery told us this was the list of fields to use, and then we can. You know reevaluate that later go blame them as necessary.

A

Okay, sure um uh yeah.

I

All right yeah, I can. I can add that in the comments uh you mean in the comments kind of like when we turn on the when we offer the flag or.

A

Or I I meant I meant just like the code comment like this is an allow list of properties that are of of properties of object. Meta just like if the package crd known types file.

I

I

Sure sure I can I can add that but yeah I wanted to bring this pr to everyone's attention. I see that like end of january, um so I and I wasn't getting much love on the pr, so I wanted some feedback to see if anyone, but I can definitely uh add those comments on how we came up with this list of fields.

A

Yeah apologies for the review latency. My review latency has been kind of terrible recently. It's.

I

Fine, actually, uh I think I I sent a mail to my list and someone replied with an invitation to these meetings. I was like that makes a lot of sense. I should go to those meetings all right.

A

uh All right- and I think it looks like you- have the last one as well. um Yes, I.

I

I sent another pr uh which adds another feature flag uh well or how to call it a marker sorry. uh So this marker is to add support for deprecation uh right now. If you have multiple versions in a crd version, one versus version three and the the official definition has a feel for marking a version as deprecated.

I

um So this one pretty much does that right. So if you pass this marker on a version of your cid uh as it's getting built, the deprecated warning is going to actually be turned a very put, and you can also pass a deprecate message. That's also specified there uh in the marker as warning, so you can pretty much push them down to your generator's crd. This is useful for when you have again you're evolving the versions of your crd and you're already version two version. Three, and perhaps you want to deprecate version one.

I

uh That's one way that you can push uh turn on this feature again uh optionally. Alternatively, you can always manually add this to the generated crd. This is just just making it. So it's to control your tools.

A

Yeah no, this seems this seems good. We should, if there's an equivalent fields in the uh crd, we should have that. That seems this seems pretty reasonable.

I

And I think these are the two items I wanted to bring to this meeting. If anyone has any questions or feedback, please let me know.

D

Seems that no one has any question. uh I was late to the meeting, so my two points were skipped, but if you are fine, we can go through them now. If there is no other point raised, I.

A

D

A

No worries yeah go ahead.

D

Okay, the first one was related to internal internal, providing the documentation of keybuildering in other languages. There is a pull request that has been there for some months and is actually outdated so from the chinese community. That wants to merge it to provide the documentation also in chinese, and I think we don't have any example of how we want to treat them or how this should be considered.

D

Probably we need to provide a domain which will be used for that language and also, we will probably want to describe how we want the process to be, for example, uh maybe whenever uh a merge is done against book versus three, it should also be merged versus against these others uh branches, and then the communities that are in charge of of translating them be notified and translate the additional parts or things like that, and I think that we haven't decided anything in this sense.

D

So it would be nice to discuss that so that we have a common proceeding for for all this. Maybe in a future we also have in other languages like spanish or italian, or I don't know so. I think we should actually define how we want to treat that.

A

Yeah, I think the original reason I had held off on making any decisions is because I wanted to get like a review from an official like sig docs reviewer on, because I think they have reviewers that can just like go through and like give a quick scan to make sure everything is reasonable um in different languages.

A

They never got back to me, um so I think yeah. I think, probably just let's write down what you just said in in the in like a little document about how we do our internationalization. I can create the subdomains and stuff and um we have different. We can have different subdivisions.

A

Actually, I should see if there's a structure in in md or someone else can see it if there's an infrastructure for internationalization already already, because if, if there is having a little language, selector or whatever would be nice, um but and then, if, if someone here or if we can maybe put out a call, if someone can show up to a sick, docs meeting or try to poke sig docs again and see like hey, is there a good way for us to get reviews?

A

um Because, like the main thing I'm concerned about is like you know, I I I can't do reviews in chinese um for for doc reviews, and so it would be. It would be great to have just like someone that we could trust that could just be like hey these. These generally look reasonable for any get for any given language. We want to support whether it's you know chinese or spanish or italian, or whatever.

H

We we have one fair for the chinese stuff, but I I cannot review chinese as well, but the biggest problem is because it it's not showing the preview. I think he has something broken the change as well.

H

So maybe if we can just provide you how they can do that, because now it doesn't know if the gpr's making movie for january jesus, what I'd like to say. So if someone started to spend time trying to reveal that, maybe you know we'll lose the time.

A

Yeah, so I think I think maybe the follow-up from this from this meeting is. We should put together a little a pr in the in the repo under our documentation, instructions of like what what what the process is for internationalization of our docs.

H

A

And and how we're gonna do it and what people can expect um and then we can go from there.

D

Yeah sounds reasonable, sure sure and asking for a review for a new language that we don't know ourselves from sick dogs should be added there to, for example, I can review the spanish version, so we didn't. We won't need that for the spanish one, but probably in other languages, not only chinese, we will need some help in that sense to make sure that we are not insulting anyone or things like that.

A

H

Yeah my initial idea was like a poche english official, because it's the language that we have the maintainers uh that you have about the king check and to the other, like not officials, something like that suggests which clarifies that you don't have any responsibility and today, if you will not to be able to get help from specific kitchens, you know just to let they push the chains.

H

I hope you'll be very hair. Someone put some very bad there. Doesn't you know.

J

But I mean sig, I've been told.

A

That sig docs does have like team official teams for this, so uh you know if we can convince them to come to our repo. I you know no, not only this should be. This should be something that's that's nominally covered by those groups and sig docs. um So I think we just need to figure out how to hide. I mean.

D

Having a separate branch per language and merging there, every time we merge into book b3 also sounds reasonable and then committing just to that branch. The changes in that language should, I don't know how hard it will be to resolve the mergers once the chinese branch has an additional commit with the chinese documentation and we want to merge uh the additional documentation that was created in english, so that the chinese also has it, and then they have to commit again to to translate that.

D

I don't know how hard it will be to resolve those uh merged conflicts.

A

I'm I'm curiou, I'm actually I'm curious to see. I don't know what upstream does in terms of this. This workflow um I'd be curious to hear what they see what they do as well.

H

Yeah maybe try to to look in after a better solution. Maybe it can be a follow-up as well.

D

Okay, so that was the one of the points and the other point that I raised. I don't know if anyone has any other concern about this or.

D

Okay, so the second point that I raised was about the version three stable.

D

I think we have merged everything that we considered blocking for version three and we already have a and a better one, the second beta of of this release. So I think that we should like uh define a schedule or a calendar where we are going to release a version 3.. Maybe I don't know if we want to release a release candidate first or directly version 3.

D

I don't know if it's gonna be in one week, two weeks a month or a year I don't know, but I think we should try to to make a calendar to to follow those those those steps.

H

About the very sound three, what do you think about? Shall we releasing more more one beta after next week and today after that, the stable one, because it has a few details that I saw just let's see? Maybe would you be better before, but it's small things of course it's it's not mandatory, but it shouldn't be nice.

A

Yeah we could, we could do a beta or we could do a release candidate. I mean if it's, if, if that is the release candidate, if we think that's going to be the release candidate, then we might as well just do it as a as like dash rc dot zero, um but yeah. We could do one more pre-release release. That's fine! I think.

D

Okay sounds fine, for we can like try to schedule the next release candidate for next um thursday in in a week time and uh in the next q builder meeting in two weeks uh make the the final release. If everything went the right way, though,.

H

Sounds good to me.

D

H

Can be like says, oh, I think you're one week after it doesn't know.

A

Do wait? Do we want to try to do the release candidate today actually and just to give people two weeks.

H

King, it's all day, I I prefer next week because it has just a few changes that I think would be nice to get to marriage before uh just forever be sure uh small things, uh and today, yes, can be on.

D

Monday, maybe on monday, yeah yeah.

A

Monday monday seems fine. I just want to give people like one week to two weeks. Two years to test is probably a more reasonable release. Candidate duration.

D

A

Just just one week, okay sounds good to me.

A

All right uh does anybody else have anything that they would like to talk about.

A

Today sounds like not uh all right, so I will see you all in a couple weeks.

H

Really. Thank you. Thank.

I