Jenkins Platform SIG, 7 Aug 2020

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: 2020 08 07 GSoC Git Plugin Performance Project

Description

Jenkins Google Summer of Code performance project meeting August 7, 2020. Topics included the GitToolChooser and the pending pull requests to the git plugin and git client plugin.

A

Yes, I'm going to share my screen.

A

Hi everyone, today's agenda, the first thing is okay, so um so I've done some interactive interactive testing. With uh with some unsupported extensions, I've tried adding those extensions and to see how our gate tool chooser is behaving uh when those extensions are added and fran specifically asked for. uh For for those results, we were having a discussion in the pr, so I think we can run a build and see it in real time. If that's so in this project, I have a very small repository it's under 5 mb.

A

So so I go to the configuration page.

A

So this jenkins instance uh contains both git and jget in its environment. I have specifically chosen jegit as the preferred tool and so right now I have a I have an additional behavior, I'm going to delete it, and then I'm going to run this project build this project. So what should happen? Is it should um it should pref it? It should prefer jget and the recommended tool should be j get by the get tool chooser, since the repository size is less than 5mb, which is the expected behavior.

A

Now, as we go to the project again and we configure the project and we will add one or two extensions, additional behavior which are not supported by jegit. So now, after adding those extensions, um theoretically, the the git tool to the chooser should not recommend jk. It should recommend get.

A

That is what it should do. So.

A

So so timeout is also a feature which is not supported by jegit. So let's just save this and I have to delete the wipe of the workspace.

B

Now why do you have to wipe out the workspace.

A

Because I am, I don't have to because I am. If I don't, then it is not going to check out the uh not going to go through the one of the functions I want it to go through. So if it's, if it's available, then it doesn't, uh it applies a different operation right.

A

So yes, here the recommended tool is get so so I tried this with right now I have shown it with timeout. I tried it with clone behavior, where I uh included shallow clone there also. I saw the same thing, so I would assume that functionally the unsupported command is working, how it should and the way we are taking that information.

A

It's working, how how I expected it to now. I think, with the with this current feature: unsupported command and the get tool chooser. What remains is uh testing in different scenario. I would say: uh would you recommend an exhaustive list of all the extensions which are not recommend which are not supported by jigget and then testing all of those extensions before and after cases?

A

That is what we would want for each of the extra extensions which are not supported injected right.

A

I have done it with two or three.

B

It's it certainly seems given how many people are using the get plug-in it seems safest to check as cur. Certainly if there's an option in the plug-in, it's probably not only been used but likely misused, and- and so I'm prone to say yes but okay, but I'm open to other other discussion or others saying oh no, test 50 test, you know test test a random selection, uh the the spot checks you did are great. I would love a broader check personally, okay.

A

B

Fran fran your comments, your concern, because it's you're asking a very valid trade-off question we're trading time to do interactive tests for other things right. It's it's a valid, a valid trade-off question.

A

C

A

Yes right, sorry, no.

C

No, no, I I was allowed to say that, no, because most of my questions were all about the question that with your test, have been answered so now I have a clearer understanding of your pr. So now what I want is to review again the prs and be sure that now I have all the information and I'm in a better position to review it all of them.

C

A

Okay, so um the second thing is a change in design. So now what was happening before the change so for get tool? Chooser, we had two constructors, the first one uh used git s, a git sem source object to get the cache and it relied on the source. Git same source object to get the cache and the second construct, and and this constructor we uh so what we did was. First, we look for cache if we could find it and we estimated the size and recommended the tool.

A

If we could not, then we look for extensions, and then we asked for the for the size from them and if we could find that, then we could recommend the tool now.

A

The second constructor I gave was an option if we just have the remote name and nothing else so that it can be used across other projects apart from multibranch project. So now mark suggested that we would. We should not need the git sem source object to get the cache entry, and um I I did find that that we would only need the remote name to access uh any dot gate directory uh cached within the jenkins instance. Workspace so now the uh we don't need two constructors.

A

We just need one constructor, which would require the remote name, and then it would make the decisions, so it has two ways to find the size. The first one would be the best way we have is to determine it using cache, and if we don't have that, then we use the api, a rest api method.

A

So and if so, I ran the unit tests again on this to make sure that the code is working with just the remote name. It doesn't require the source object and it is working well, it's all the cases have passed, so I don't think we would need much.

A

I would I haven't pushed this code on the pr yet I just wanted to show you first. So I think this is it with the design modification.

A

I I think we would need more design modification, I'm coming to the third point now, so that is related to the support of other extensions, so I've been uh looking at uh so the first question I have with the support of other extension plugins is: do we first in in our first go? Do we want to only support public repositories and size related to public repositories, or would we want to okay, so we want both of them.

A

So uh for that uh I I asked in getter about user authentication and uh mark suggested that it would be best that we allow get the brand source plugins to decide uh what kind of credentials it should use to authenticate the user and then use the api to get the size of a private depository if it wants to. So uh with that approach, I I was looking at the brand source implementation. I have some concerns.

A

The first concern is that so the way uh the brand source plugin is going to uh pass the credentials or get the credentials is, is the same as I I found in the git plugin, it's using the lookups scan credentials functionality, and so it needs three things. The first is the context that I assume is the job which is running. The second is the api uri, which is what we're providing. The second is the credentials id.

A

So, as far as I understand the the two things, these two things is something we have to provide to those uh plugins. This is not something we can take from them uh because, whatever so our rs, yes, yes,.

C

One question about the credentials: why is git client who has to provide that credential to the other plugin? I don't know exactly what is the the? How how other plugins are using the plugin in, for this very case a concrete case? Okay, that's what I'm I am asking, because my understanding is that ah no okay! No! No! No! No! No! Okay! No! No! I got it. Nothing! Nothing! Sorry! Sorry for the mess.

A

No actually, I I tried that I I tried so my first implementation approach was to use the context they have or to use the credentials id, because I thought that they, if the source plugin is installed. In my instance, I can use the the context and the credentials id this uh project would have, but that would be. That would not work, but still I so. I wanted to do that.

A

But then I figured out that I found that the extension is implemented as a static class and all of these, um whatever objects, we're trying to access, are non-static.

A

First of all, I could not figure out a way to get them in that I don't understand. Is that is that possible, because if the, if these are static classes, that that means that they're going to be shared across across all the objects which is not, which doesn't make sense with the credentials id, because that would be linked to a particular object.

A

So that's what I understood from it. So I thought that it would be best for us that the git plugin, while it's asking for a particular size for a particular repository, should also tell what project it is. I I have a very limited understanding of the uh class called item. That is what it is. Accepting uh item item is the context it wants and then the credentials id is something we can provide and then we can then it's it's.

A

The the job is, I think, is easy, but now I the question I have with this approach is uh related to the item, the job so right now, when I did not pass any credentials, I my uh the git plugin was you was bill was using um so I was using a freestyle project and I and I used the api extension of brand source without it using it anywhere that so my my question is: if I want to pass it credentials, which means that I'm passing it a context, does the plugin, which is the brand source plugin needs to be needs to be used in a project?

A

Is it necessary for it to be used in a multi-branch project and if it's not being used, it's just there it's in inside the jenkins instance and has never been used. Would it still be able to accept the context I have and use it to scan the credentials and connect with github.

B

I think it's an excellent question. I don't know the answer, but I may have a specific example of a case that will will likely fail and and need some more thought.

B

So so, if imagine a freestyle job, that is a credentialed a job with a credential that uses an ssh private key as its credential.

A

B

All right, so that's the credential id we have, and now we ask the scm repository scanner from the branch source plugin to scan with that credential id. It can't because rest apis are not secured by private keys, they're secured by a username password or by tokens. Okay, and so so I think I think your your question is a crucial one and I don't know the answer to the question.

B

I'm hopeful that the branch source plugins will see the dubious credential id, ignore it or or bias towards choose a credential that they already have associated with the a particular repository. But I don't know that for sure.

A

Okay, so that is something I because they they do validate the api uri we provide. So I haven't looked at how they're validating it. Maybe they are doing what you are suggesting here, but I guess.

D

This is the problem we're trying to solve the to allow this for freestyle jobs. Is that.

B

Well, it's to allow it for anything, because we we want the tool chooser to not to not require that you must be a multi-branch pipeline right. The tool chooser needs to benefit everybody. We would like it to benefit everyone and and therefore, if, if it can ask the question viably for the branch source, great ask ask the question to get the answer.

A

So what I can see from how they're validating their api url they are checking if the credentials is an instance of standard username, password credentials. If it's not, they are throwing an io exception, saying unsupported potential type. Okay,.

B

So that you've already answered the the first first observation I had if we inadvertently pass a private key credential to this check, api url validity will fail.

A

And that means that we will not be okay.

B

Right, but that that's it's it's perfectly they they may not have anywhere in their system and a token for the branch source right. They may have just installed the branch source and have no no username password assigned to it. So so failing is a valid, valid behavior.

E

I guess the other thing.

D

Is how would you, how would you associate, I guess, are you doing a lookup on the on the url to determine that it's github, because the branch source plugin? I think it knows that it's github, because your yours you've configured that as part of your multi-branch project. So in a freestyle context like how would you know that you would talk to the github branch source, plugin versus the gitlab branch source, plugin.

B

I thought he just asked every I thought you were just asking every provider, do you have one? Can you answer me this did I misunderstand rashad.

A

Yes, yes, that is what so, we've used x2 exposed method, which is this is the url applicable to your plugin or not so if they um so there in that process, they can validate it using the uh api, your validity, this method or, however, they would choose to so um then my question is so for, for example, what I so what I experienced while I was uh sending uh unauthenticated uh requests, was that I was in a freestyle job.

A

I was able to get the size of the repository without having a cache, and I did not have a multi branch project within the jenkins instance. It was just the only project, so with authenticated requests, we would need to figure out a way that um the credentials are either the username password or their token.

A

That is that is those are the two types of credentials which would be supported by a rest api. So so then, so, if the freestyle job bud is configured by the user, as um with the private ssh key, then we we cannot do anything about it right.

B

Right, I I don't think well, no, maybe that's too strong, because it's there are some of the branch source plug-ins that have have various mappings. That attempt to. Oh, you asked to use this ssh key and I have a username password pair that I can use whenever you mention that, so so, there's at least one branch source that says hey. I can do a checkout with private key, but still use this username password pair for for token based access for for rest. Api calls.

B

So I'm not sure that you should optimize away the call to the branch source. We should ask them and let them say no.

A

Okay, okay, so so, then, um then, when we are sending the repository url, for which we need the size we, but we would send the context and the credentials id. That is something we have to send. I.

B

I think you're right, I I wish it were different, but I think you're absolutely right the thing that knows that I had the flawed idea earlier and it was obviously completely flawed that gee this the branch sure should just know the credential it will use. But really it can't right because it has to know has to know the url and it has to associate the the credential with the url. So I think you're right, I think we're going to have to pass it.

A

uh Or maybe I I haven't explored the brand source plug in any other brand source plugin where it cannot it contain. A map could not create a mapping between the repository, ui, urls and the credentials it has stored. It would right it would create a mapping. So if, if I am, um if I'm sending a repository url, it would be able to check if it contains the credentials.

B

Yeah, it's it's certainly worth exploring right. It is absolutely worth exploring to see. Do we really have to pass a credential, but I suspect, you're going to find? Oh, yes, we have to pass a credential.

D

Okay- and I guess I'm kind of wondering like because you wouldn't want to accidentally pick up the wrong credential either, because that could like lead to credential leaks.

B

Right right, that's a novel one right! Oh, I asked to scan the super secret everybody's passwords repository and I just borrowed the credential scanning the credentials from from the jenkins system.

D

Yeah, I wonder if you need like if it ends up not being a uh if it ends up not being a token or username password like I wonder if, like for a freestyle job, if you'll end up needing to provide an option for the user to select their rest, api credential or something like that, I don't know anyways I'll. Look at the prs. Sorry, I'm behind too! So I'm I think. I'm slowing us down.

A

That's okay, so I so what you're suggesting is that if we, if we know that the credentials the user has passed, would not be uh able to use for the rest apis, we would ask the user if they can provide.

B

uh Yeah, for me, that's that's a very exotic case. I wouldn't worry about it. Rishov is this is just a heuristic right. We we know it's a fallible rule. We know it's it's going to to not always get it right, it's perfectly okay to say yeah. In that case, we just didn't get it right.

D

I think what I proposed was maybe like some later thing. If we can't sell it correctly or something like.

B

That so yeah telling telling users they need to modify their freestyle job in order to get benefit from this feature is probably not quite as dramatic as telling them. They need to upgrade the plug-in, but it's it's it's. It's still rather dramatic, saying. Oh in order to use this, you have to upgrade the plug, upgrade the plug-in. That's fine, but oh, and you have to touch every one of your thousands of freestyle jobs.

A

And this time, okay, so this is something I have to explore more now and uh okay after that, which I think these are the things I needed to discuss. um I we have three pr's three open pr's, which fran has reviewed the get to chooser and the test extensively. I uh so I think it's if the other mentors can review it and then we could merge it.

A

So of course, I'm going to push the new the change in design which, where we just need one constructor, so I I am going to make that uh push that in about after the meeting I'll do it. So then I uh in terms of get tool chooser, it's complete!

A

Oh sorry, I'm sorry! I just forgot the extensions uh it wouldn't be complete. It would work for extensions uh with only get a brand source plugin and with public repositories, not even for private repositories. That is how it is created right now. So if, if it's important for us, if it's critical for us to first implement it with private repositories as well, then this pr will be blocked till um and figure out a way how to manage credentials and how to pass them.

A

So that's a decision, maintainers and the mentors can make then the other two pr's we have are related to the unsupported command feature that is checking if the extension is compatible with jgit or not and then recommending a feature, an implementation. So so I need to add test cases for the unsupported command class. I I haven't I've interactively tested it not added the unit test cases.

A

So yes, this is the current status.

D

I have another, maybe naive, question and uh so forgive forgive all the questions that may have already come up in pr's. uh I'm sorry, I'm behind on the pr's also, um but one question I have is uh for public repositories. If you use api tokens too often, then I think that you get start to get throttled.

D

um Will that have a negative effect on us.

A

um So they they. What you're talking about is the api uh limit right, tell me yeah. We have so uh so the brand source github brand source plugin, I explored. They have a function which checks the api rate limit every time uh trying to make a request. So I think we can. We can just uh we can add that function and just we can hope that requests not being so. I, as far as I know it's for a for an unauthenticated user. It is um 50 times in an hour and I'm not I'm not.

A

I I didn't it's.

B

A

It's low, so that would be a problem. So yes, yes,.

B

Go ahead, I shouldn't interrupt continue.

A

No, I I was just saying that I at the time of doing it, I I thought that let me just think about uh figuring out figuring it out for once I don't and let's we'll think so. I was thinking that we would at some point of time, uh implement the authentication and that would remove uh the rate limit question. So, yes, justin, that's a very valid question and we cannot do anything about it.

A

It would be a concern if we have a lot of projects which are trying to actually it would be a big concern if we have a thousand freestyle job projects which are trying to, um but then they would would have to have the same repository url as well. In the same instance, I'm not sure how this would affect us.

A

So if, if a particular, as far as I know, so, how would github know that the same particular machine it would map it with the ip right so for a particular machine's ip? That is how it knows that okay, this particular ip, is sending more than 50 requests in an hour, and then I.

B

Think so can't we just can't. We just rely on the branch source to manage the rate limit. uh It seems it seems, like the github branch source at least has heroic, and I I do mean heroic. It's got two different choices. I can either choose to use a. What is it I can use a linear barrier, or I can declare that that I don't want to block raid api rate requests.

B

Api requests until I'm very near the threshold and and so they've got they've got an awful lot of intelligence already inside the branch source that branch source anyway, I would, I would just rely on them to do it. Now. It's a valid point, though I wonder given given the given the relatively insensitive nature of our request for the size of a repository.

B

Should we consider a cache somewhere globally, which a little hash table, which remembers we've already asked for the size of this repository? Don't even bother asking again rashab. Is that already happening or every time a repository url is referenced? You ask the question to the to the to the rest, api provider.

A

um Every time we ask for it, we we do not maintain a cash or do not maintain.

B

Any kind of information that may be something we might consider just because we know that the the size of the repository is unlikely to decrease and- and it's also unlikely to take dramatic increases in sizes in any given 24-hour period right and if it does, we made a poor choice that time and we'll get better next time.

B

So so something like uh remember remember, and if we asked in 24 hours, don't bother asking again is not unreasonable and it's a way of avoiding you know. Oh, we already asked: we've lived for a long time. We can. We don't have to ask again.

A

That's that's a great suggestion. I uh okay, so uh a table which maintains uh the sizes for a 24 hour.

B

Yeah something the hash map right, I mean it's, it's not this. Isn't this isn't a big data structure right? It's we've got a repository, url.

B

Although we can still see his screen.

D

Yeah, it looks like it looks like the power went out. Maybe oh man.

E

So uh I was just navigating through the uh rate limits and I found one uh deprecation notice that github will be discontinuing the password authentication.

B

Well, so what what they're discontinuing is the use of your password as a credential, but but you shouldn't do that anyway. So it's a good thing use.

E

It yeah they're completely shifting to the api right. That's it.

B

Hey richard's back okay,.

A

I had a power cut so okay, so I was just saying that that's a great suggestion mark and I I would definitely implement that so uh my question is: would we want that in this pr or can we uh have.

B

Another pr for.

A

B

Later, yes, that's you're welcome to go much later, that that is, that is optim optimization after we've learned many more things.

D

B

D

Think my concern is just like- I think you would just want to handle like if the github plugin branch source plugin thinks that it's going to hit a rate limit. Maybe it should just say, like I'm, not going to give you the size or something like that. You know something simple that would take care of and not make the performance worse.

D

It's kind of like the idea, and maybe you've already considered that and that's already being handled.

A

No, it is I've just expected that it throws an exception. I will just say that: okay, we can, we cannot give you the size, that's what I have done, but that's a very valid uh point, and I would so with the I.

B

Might already handle that.

A

The exception would, but yes, uh so authentication is what we have to look, uh and uh the last question I have is um so right now we have the get to chooser as a pr, but we um I haven't in I haven't. I haven't, put the instantiated the tool chooser all over the kit to choose get plugin wherever we are creating a client.

A

So um so what I wanted to ask was, would we want different prs for uh places where we are instantiating different places where instantiating the tool chooser, so that we can focus on those areas specifically and not just in one pr way? Where everywhere we have the instance, and maybe we miss some particular edge cases where um the chooser would break cases or or is that is that, okay, a pr a single pr where we have all the places covered where the get tool chooser could be instantiated for the.

B

Client, I asked ask your question again: I'm sorry richard and help me focus.

A

It's not uh so. The question is that would we need different pr's for different places in the git plugin, where we're going to instantiate get to user, or would it be comfortable for us to review it in just one pr this for the current pr? It doesn't instantiate they get to user anywhere. It's just introducing the class within the plugin and the tests.

A

I have folded one at one place so within the git scm checkout.

A

Or at other places on the planet yeah am I.

B

Audible, uh just barely you're a little garbled, so one pr or multiple pr's you you get to choose that. I I don't know that I can see one being better than the other.

B

It's we know we know we've got prs to merge on get client plug-in. We know we've got some to review still and merge on git plug-in and it may let you work go forward in somewhat independently. If you aren't inside those specific pr's, but you get to choose, I I hope sincerely to review the prs this weekend. I apologize that I haven't still haven't done. My due diligence.

A

It's okay, it's! Okay! So um that's it from my side. I think I'll be looking into authentication how we're going to support the extensions credentials and more discussion on the guitar channel if, if I'm stuck somewhere, yes, so thank you. Everyone for coming thanks, rishabh, bye, thanks, bye,.