Add caching for fluxv2 plugin (first step toward ##3032) #3044

gfichtenholt · 2021-06-25T09:12:40Z

This is just a first stab at #3032, just to get the electricity flowing from A to B, as it were. Introduces redis dependency and does some basic writes and reads to/from redis cache.
Plus tiny clean up - I saw some code in asset syncer that would be better to move to http-client util

gfichtenholt · 2021-06-27T08:27:11Z

making some significant changes to do with unit testing the code. Gets a bit messy due to several reasons:

current clientGetter impl assumes a Bearer Token in incoming request context. If I am starting a background goroutine, I don't have an incoming request, so I need to either
a) not use clientGetter func at all, in which case I need to define something similar to solve the issue with server side/client side unit test context OR
b) hack something to get around the current limitations, which is what I did to get past this
need to fake (mock)server-side redis client, just like fake k8s client. Gets pretty detailed and verbose
current fake k8s client doesn't handle watches "as is", which is the functionality I rely on. In other words, fake k8s client does not act the same way w.r.t. watch events as the real server does. Need to play with it a bit more to see if I can make it work.

So, long story short. please wait to review until I mark it as ready for review again. Thanks

absoludity · 2021-06-29T01:36:25Z

OK, I'll not look at the code for now, but just wanted to be sure you saw my comment on the issue (#3032) last week (didn't see a response, so wasn't sure). Not that it needs to change anything, but I'd just tried to think through and discuss some points there.

gfichtenholt · 2021-06-29T07:55:23Z

yes, I saw your comment. Read it multiple times, as a matter of fact. Everything makes sense and all will eventually address all of it. I am not blocked by anything at the moment. Just mocking everything in unit tests takes a lot of effort. I have to manually mock k8s server firing events (see kubernetes/kubernetes#54075 for details) in my unit test code, and every single call to redis being made. It's gross beyond belief, but I will be done with it eventually

gfichtenholt · 2021-07-01T06:16:55Z

ok, this can be reviewed at this point. All tests are passing, at least on my end. Keep in mind, this is only the first incremental step toward a big feature. MANY MANY changes are to come incrementally. Among them:

get rid of nasty client getter hack in cache.go
investigate how to handle updates to index.yaml that flux does in the background
These need to be addressed before I can move on to longer term:
eventually the cache engine will be generalized and most code moved out of flux plug in to core server, except maybe for one or two functions relating to what specifically is cached and how updates are handled, so move towards that.

absoludity

Great stuff Greg! I'm not yet half-way through, but just posting thoughts so far. On Monday I'll pull the branch and try running it. Very excited to see this getting closer :)

absoludity · 2021-07-02T02:35:06Z

chart/kubeapps/templates/NOTES.txt

@@ -65,11 +67,20 @@ To access Kubeapps from outside your K8s cluster, follow the steps below:
 ##########################################################################################################
 {{- end }}

+{{ if and (not .Values.redis.existingSecret) (empty .Values.redis.redisPassword) -}}


For now we probably want to condition this note so it only appears when the kubeapps apis service is enabled?

absoludity · 2021-07-02T02:40:20Z

chart/kubeapps/templates/kubeappsapis/deployment.yaml

@@ -85,6 +85,16 @@ spec:
          env:
            - name: PORT
              value: {{ .Values.kubeappsapis.containerPort | quote }}
+            # These env vars are required by the plugins for caching functionality


IMO we should document this as required by the kubeapps apis service (which will longer-term pass something to the plugins so that the plugins won't need to know these details) though I understand that currently they'll be used directly by the flux plugin. If you prefer to remember to change it later, no problem.

I will leave a TODO comment to state this

absoludity · 2021-07-02T02:47:05Z

chart/kubeapps/values.yaml

+redis:
+  ## @param redis.enabled Deploy a Redis server to satisfy the applications caching requirements
+  ##
+  enabled: true


I'm not sure why we need a redis.enabled in the chart values given that it'll be conditional on featureFlags.kubeappsAPIsServer I think? (that is, if the apis server is running then redis should be running?)

Also, rather than adding this new section to our chart's values.yaml right now (which could be confusing for users), can you pls add it to the docs/developer/manifests/values.kubeappsapis.yaml where we've also got the values for the service itself, which we'll move back here into the chart's values once we're ready?

absoludity · 2021-07-02T02:58:14Z

cmd/asset-syncer/utils.go

-		req.Header.Set("Authorization", r.AuthorizationHeader)
+	reqHeaders := make(map[string]string)
+	reqHeaders["User-Agent"] = userAgent()
+	if len(r.AuthorizationHeader) > 0 {


So it looks like the change you're making here is related to using httclient.GetStream below, but I'm guessing you had a conflict when merging master due to the bug fix @antgamdia landed with #3057 . But if you look at the change here, I think you missed incnluding the new conditions that were a part of that change, when resolving the conflict?

What's not clear to me is what functionality you've actually changed in this file (other than the accidental reverts). It seems to be just switching to use httpclient.GetStream() without any benefit? (Ah, because what I'm not seeing in this context is that GetStream is probably doing something extra that wasn't done before?).

oops, indeed, I messed up the one line in 'if' statement when merging. I will it back to what Antonio intended. And yes, the goal of this little change, which BTW has nothing to do with caching, was just to move more http-client specific functionality into pkg. The benefits are potential code reuse by other pieces of code. Streaming response should be a fairly common use-case. It just an a "little" thing I noticed while waiting on other things.

absoludity · 2021-07-02T03:02:12Z

cmd/asset-syncer/utils.go

-		return err
-	}
-	b = buf.Bytes()
+	imaging.Encode(&buf, resizedImg, imaging.PNG)


Similar here - #3057 improved this code by handling the error which has been reverted here.

again, my bad. consider it fixed

absoludity · 2021-07-02T04:46:54Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

+
+func newCache(clientGetter server.KubernetesClientGetter) (*fluxPlugInCache, error) {
+	log.Infof("+newCache")
+	REDIS_ADDR, ok := os.LookupEnv("REDIS_ADDR")


Fine for now, but when this is moved out of the plugin, then small preference for reading all config in the main.go (whether from env vars or cmd-line options) only in the one spot and passing explicitly to functions (so functions are less dependent on env state).

I will add a comment so I don't forget

I did add a comment in the code, as promised. Just wanted to also mention that on a second look, I don't necessarily agree. The way it is right now, is one place that reads env vars. What you're suggesting is to move this code out to server.go (kind of like helm plug-in server.go reads ASSET_SYNCER_* env vars today) or main.go? So then we'd each plug-in that wants cache read these env vars? That doesn't seem right. I think I am missing something. Anyway, we'll discuss this on a call I am sure.

cmd/root.go - as in, the root CLI command would read all config on startup. If you look there, you'll see we already do this for the clusters config path and pinniped proxy URL, populating a server options which is passed on to server.go where the server is started. Note that the library we're using there (cobra/viper) is already reading in env vars that match the configured options, so you can supply a --pinniped-proxy-url or you can specify it in the env, either way, it'll end up in the value for the option.

So then we'd each plug-in that wants cache read these env vars? That doesn't seem right.

No, each plugin would just be passed a function or interface that is already configured with the env vars. Not suggesting this can be done now, just when it's generalised for all plugins to use. This is much the same way that we currently create a function closure to be passed to plugins so they can create a k8s client without needing to know anything about the pinniped proxy configuration.

I think I am missing something. Anyway, we'll discuss this on a call I am sure.

Yep, and it won't be relevant until we're generalising the caching work for all plugins anyway.

ok, sounds good

absoludity · 2021-07-02T04:51:51Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

+		redis.NewClient(&redis.Options{
+			Addr:     REDIS_ADDR,
+			Password: REDIS_PASSWORD,
+			DB:       REDIS_DB_NUM,


Ooh, interesting - wondering if different plugins could/would have different dbs? (Sorry, that's unrelated to this PR, just thinking out aloud while reading).

yes, I was wondering the same exact thing. Instead of a 'namespace' which redis doesn't really have a concept of, use a different DB. Will need to see what the cost is, if any, of having multiple concurrent DBs

absoludity · 2021-07-02T04:52:49Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

+	}
+	go c.startHelmRepositoryWatcher()
+	return &c, nil
+}


Nice and clear!

absoludity · 2021-07-02T05:02:26Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

+		c.watcherMutex.Unlock()
+		log.Infof("watcher already started. exiting...")
+	}
+	// we should never reach here


Won't this point be reached if startHelmRepositoryWatcher() is called a second time? Ah, so I assume the assumption is that it is only ever called once, but in that case, I'm confused about the locks.... maybe it'll make more sense as I continue on :)

yeah, I mean we should never reach here under normal conditions. if we reached there something bad happened. I will update the comment.

absoludity · 2021-07-02T05:05:01Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

+
+func (c *fluxPlugInCache) newHelmRepositoryWatcherChan() (<-chan watch.Event, error) {
+	// TODO this is a temp hack to get around the fact that the only clientGetter we've got today
+	// always expects authorization Bearer token in the context


It always expects the bearer token if it's not setup to use the service account. Right, so we need to ensure that the client can also be used with an explicit request to use the service's service account right?

Not quite. I am going to say that my testing shows that the bearer token is ALWAYS expected with the current impl of client getter
https://github.com/kubeapps/kubeapps/blob/master/cmd/kubeapps-apis/server/plugins.go#L307
I could be wrong, but the call to extractToken() is the very first thing that happens and that's what fails in my case. If I am right, as I said before this is going to take some doing. We either need to enhance clientGetter somehow so I don't have to hack it like this or come up with some new kind of 'client getter' that can be used for my purposes, that supports both server-side and client-side (fake with unit tests)

That's odd, in that if you look at the implementation of extractToken() just below your link, you'll see it at least intends to return an empty token with a nil error if there was no authorization in the context:

https://github.com/kubeapps/kubeapps/blob/master/cmd/kubeapps-apis/server/plugins.go#L353

I haven't checked, but perhaps context.TODO() doesn't include any metadata at all, and it's just that metadata is requried? (in which case we should change the code so it is not, which is easy enough).

In your case, you do actually want to use a context (as you're passing it to the Watch call below. So I'd not use context.TODO() but context.Background() below.

this is raising an error in my usage

md, ok := metadata.FromIncomingContext(ctx) if !ok { return "", fmt.Errorf("error reading request metadata/headers") }

I will change my hack to something a little cleaner like

ctx := metadata.NewIncomingContext(context.Background(), metadata.MD{})

Yep, or we can easily update the extractToken() to return an empty token with a nil error if there is no metadata with the context. That'd be cleaner IMO, and consistent with its current behaviour.

ok, will make this change this as part of this PR

absoludity

Sorry for taking so long to get through this. I'm happy for this to land but have left a bunch of thoughts.

The biggest one though, which may be a bit of a blocker - but better to deal with it now than later: I know we'd all talked about redis as an example for the caching layer, and you'd even asked in the past why we use postgresql for caching with the existing helm, but looking at this PR made me realise that cached data in redis may not be easily queried with filters or sorted?

For example, if this is used to cache the bitnami repo, will it still be a problem to request all pkgs available that begin with "apach" ?

absoludity · 2021-07-06T00:58:29Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

+
+	// clear that key so cache doesn't contain any stale info for this object if not ready or
+	// indexing or marshalling fails for whatever reason
+	c.redisCli.Del(c.redisCli.Context(), *key)


Wouldn't the cache want to have the most recent value if the next update/index/whatever fails? Seems odd to create a time period where there isn't a value for it like this until it does possibly succeed later?

Sorry for taking so long to get through this. I'm happy for this to land but have left a bunch of thoughts.

The biggest one though, which may be a bit of a blocker - but better to deal with it now than later: I know we'd all talked about redis as an example for the caching layer, and you'd even asked in the past why we use postgresql for caching with the existing helm, but looking at this PR made me realise that cached data in redis may not be easily queried with filters or sorted?

For example, if this is used to cache the bitnami repo, will it still be a problem to request all pkgs available that begin with "apach" ?

Shouldn't be a problem with the right abstractions, Redis does have rich query capabilities.
https://redis.io/commands/KEYS
The example you gave should be fairly trivial to implement. Remains to be seen as a whole the query syntax as rich or richer than SQL but I'm sure there's enough there to get us started.

Wouldn't the cache want to have the most recent value if the next update/index/whatever fails? Seems odd to create a time period where there isn't a value for it like this until it does possibly succeed later?

my rationale at the time was to better to have a cache miss than return a out-of-date value? I am willing to be convinced otherwise :-)

Shouldn't be a problem with the right abstractions, Redis does have rich query capabilities.
https://redis.io/commands/KEYS
The example you gave should be fairly trivial to implement. Remains to be seen as a whole the query syntax as rich or richer than SQL but I'm sure there's enough there to get us started.

I guess you mean SCAN rather than KEYS (KEYS is for debugging, non-prod use only, afaict)? Checking the guarantees of SCAN, one point in particular (about elements being returned multiple times) could be an issue? See what you think.

my rationale at the time was to better to have a cache miss than return a out-of-date value? I am willing to be convinced otherwise :-)

Up to you, I was just surprised because elsewhere I saw you explicitly weren't associating expiration with data, so seemed odd to do so here.

Shouldn't be a problem with the right abstractions, Redis does have rich query capabilities.
https://redis.io/commands/KEYS
The example you gave should be fairly trivial to implement. Remains to be seen as a whole the query syntax as rich or richer than SQL but I'm sure there's enough there to get us started.

I guess you mean SCAN rather than KEYS (KEYS is for debugging, non-prod use only, afaict)? Checking the guarantees of SCAN, one point in particular (about elements being returned multiple times) could be an issue? See what you think.

my rationale at the time was to better to have a cache miss than return a out-of-date value? I am willing to be convinced otherwise :-)

Up to you, I was just surprised because elsewhere I saw you explicitly weren't associating expiration with data, so seemed odd to do so here.

just FYI, I am not sure SCAN is the right answer here. I see the redis go client does have
https://pkg.go.dev/github.com/go-redis/redis/v8@v8.10.0#Client.Keys
which looks like it invokes the KEYS command. It does take a string pattern as an input argument. Nowhere in go client-related documentation (or code) does it tell you it's not meant for production use. I do see a warning about it on https://redis.io/commands/KEYS. I am not sure how relevant that particular warning is for us and our usage. I think it applies to VERY LARGE databases. It does say right above it "While the time complexity for this operation is O(N), the constant times are fairly low. For example, Redis running on an entry level laptop can scan a 1 million key database in 40 milliseconds.". Have we ever encountered a use case with a million keys (i.e. repos in my use case)?

Sorry, I'm not trying to make redis look bad or say postgres is better (no interest in that :) ). I'm just keen that we ensure that we can support the current functionality. I'm only pointing out postgresql's json support because that's what we currently use, not because I'm trying to find something better than Redis. For example, see the GenerateWhereClause at https://github.com/kubeapps/kubeapps/blob/master/cmd/assetsvc/pkg/utils/postgresql_utils.go#L211 which uses postgresql's json support for querying the json data which the assetsvc caches.

ah, so another way of saying it is that our existing code base isn't portable today. We're locked in to a particular SQL vendor. Bah

Well when Kubeapps began from the Monocular project (before my time), it was originally a MongoDB document DB. Then when we had to switch away from MongoDB due to licensing issues, we looked for a reliable and trusted OSS solution that would give us the same ability to query JSON blobs. We weren't looking for an SQL solution, but rather something that made an easy transition from MongoDB. Postgresql's JSON support fit that pretty well - that's all.

fair enough. And now we need in memory DB with similar JSON support. Redis may or may not fit the bill. I am working on finding out. Just saw this https://oss.redislabs.com/redisjson/#client-libraries - there is an add-on to go client I use. I will find out if it does what we need

Shouldn't be a problem with the right abstractions, Redis does have rich query capabilities.
https://redis.io/commands/KEYS
The example you gave should be fairly trivial to implement. Remains to be seen as a whole the query syntax as rich or richer than SQL but I'm sure there's enough there to get us started.

I guess you mean SCAN rather than KEYS (KEYS is for debugging, non-prod use only, afaict)? Checking the guarantees of SCAN, one point in particular (about elements being returned multiple times) could be an issue? See what you think.

my rationale at the time was to better to have a cache miss than return a out-of-date value? I am willing to be convinced otherwise :-)

Up to you, I was just surprised because elsewhere I saw you explicitly weren't associating expiration with data, so seemed odd to do so here.

just FYI, I am not sure SCAN is the right answer here. I see the redis go client does have
https://pkg.go.dev/github.com/go-redis/redis/v8@v8.10.0#Client.Keys
which looks like it invokes the KEYS command. It does take a string pattern as an input argument. Nowhere in go client-related documentation (or code) does it tell you it's not meant for production use. I do see a warning about it on https://redis.io/commands/KEYS. I am not sure how relevant that particular warning is for us and our usage. I think it applies to VERY LARGE databases. It does say right above it "While the time complexity for this operation is O(N), the constant times are fairly low. For example, Redis running on an entry level laptop can scan a 1 million key database in 40 milliseconds.". Have we ever encountered a use case with a million keys (i.e. repos in my use case)?

Yikes. I just read redis/redis#3627
I am going to stay away from KEYS :-)

absoludity · 2021-07-06T06:21:11Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

+	var value interface{}
+	var setVal bool
+	if add {
+		funcName = "OnAdd"
+		value, setVal, err = c.config.onAdd(*key, unstructuredObj)
+	} else {
+		funcName = "OnModify"
+		value, setVal, err = c.config.onModify(*key, unstructuredObj)
+	}


Fine like this, but remember that functions can be assigned to vars just like anything else, so you could also do the following if you prefer:

// Define an actual type so you can use it in your interface earlier also, as well as below: type CacheSetter func(string, map[string]interface{}) (interface{}, bool, error) var addOrModify CacheSetter if add { funcName = "OnAdd" addOrModify = c.config.onAdd } else { funcName = "OnModify" addOrModify = c.config.onModify } value, setVal, err := addOrModify(*key, unstructuredObj)

ok, will do

absoludity · 2021-07-06T06:38:13Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

+	for i := 0; i < numWorkers; i++ {
+		wg.Add(1)
+		go func() {
+			for job := range requestChan {


Suggested change

for job := range requestChan {

// The following loop will only terminate when the request channel is closed (and there are no more items)

for job := range requestChan {

Just to help people parsing the code.

absoludity · 2021-07-06T06:40:30Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

+		}
+	}
+	return responseItems, nil
+}


Neat. As above, I'd sprinkle a small comment just for things that are less obvious if you haven't recently been using channels.

absoludity · 2021-07-06T06:41:59Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/main.go

-	svr := NewServer(clientGetter)
+func RegisterWithGRPCServer(s grpc.ServiceRegistrar, clientGetter server.KubernetesClientGetter) (interface{}, error) {
+	log.Infof("+fluxv2 RegisterWithGRPCServer")
+	// TODO (gfichtenholt) return an error when func signature is changed to allow for it


Looks like you've now done that TODO?

oops. will remove. thanks

absoludity · 2021-07-06T06:46:26Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go

-	}
-
-	if request.Context.Cluster != "" {
+	if request != nil && request.Context != nil && request.Context.Cluster != "" {


grpc compiles in getters for you which automatically return a default (empty) struct if the pointer was nil. That is, you should be able to replace the above with:

Suggested change

if request != nil && request.Context != nil && request.Context.Cluster != "" {

if request != nil && request.GetContext().GetCluster() != "" {

(not even sure if it works with a nil object to begin with, forget).

absoludity · 2021-07-06T06:51:25Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go

+	// TODO (gfichtenholt)
+	// 1. HelmChart object needs to be co-located in the same namespace as the HelmRepository it is referencing.
+	// 2. flux impersonates a "super" user when doing this (see fluxv2 plug-in specific notes at the end of
+	//	design doc). We should probably be doing simething similar to avoid RBAC-related problems


I'll have to go back and re-read that to get more context, generally not keen to avoid RBAC-related problems with "super" users, but it probably means something different. Generally the user's token should have permission to create the chart. I'll read the doc more thoroughly later.

absoludity · 2021-07-06T06:56:48Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server_test.go

+		}
+	})
+}
+


These last two are very long tests... but I guess that's due to the related state that you're needing to setup (with the cache) to actually run the test? Looks painful (for you).

it WAS painful. took several days to make work. bleh. Will see if there is some opportunity for clean up/tidy up later

absoludity · 2021-07-06T07:05:47Z

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/utils.go

+	if err != nil {
+		return nil, err
+	}
+	return protoMsg.AvailablePackagesSummaries, nil


Hmm, so we're just caching the blob of available package summaries. From a bit of reading, it sounds like we'd need to add our own indicies manually to have some sort of selection/filtering of results with redis, which might be one big reason to use a database for the cache in the first place :/ (just thinking out aloud based on what we're currently doing with the helm support).

yeah, I know. I will change what is being cached for flux. Wasn't meant to be final. The goal was to get the plumbing right.

…ichtenholt/kubeapps into add-caching-for-fluxv2-plugin

gfichtenholt added 7 commits June 23, 2021 05:36

bump fluxv2 version for local dev env

f960a76

step 2

e5f2803

step 3

17ebc92

Merge branch 'master' of github.com:kubeapps/kubeapps

15f81a2

Merge branch 'master' into add-caching-for-fluxv2-plugin

d2d6e6e

step 4

2ef6755

Merge branch 'master' into add-caching-for-fluxv2-plugin

bb096b0

gfichtenholt self-assigned this Jun 25, 2021

gfichtenholt linked an issue Jun 25, 2021 that may be closed by this pull request

Investigate caching layer for kubeapps plug-ins #3032

Open

gfichtenholt requested a review from absoludity June 25, 2021 09:16

step 5

2abe793

gfichtenholt marked this pull request as draft June 27, 2021 08:25

gfichtenholt added 2 commits June 27, 2021 04:03

step 6

ca93b41

step 7

ddfdde4

gfichtenholt added 10 commits June 29, 2021 02:00

step 8

b696ea6

step 9

65369b7

Merge branch 'master' into add-caching-for-fluxv2-plugin

c2816e5

step 10

bf84b9b

step 11

7525ac5

step 12

faa7b03

Merge branch 'master' into add-caching-for-fluxv2-plugin

212aa84

step 13

2ab84b2

step 14

b155246

undo un-intended changes from a messed up merge

5475ec8

gfichtenholt added this to In progress in Kubeapps via automation Jul 1, 2021

gfichtenholt marked this pull request as ready for review July 1, 2021 06:11

gfichtenholt added 2 commits July 1, 2021 17:58

Merge branch 'master' into add-caching-for-fluxv2-plugin

2dbb7fe

step 15

a844bb9

absoludity reviewed Jul 2, 2021

View reviewed changes

gfichtenholt added 11 commits July 1, 2021 22:56

test cleanup

2f3fb90

Michael's feedback

6727b21

Michael's comments #2

ad0fd45

step 16

fbc79aa

step 17

4121826

step 18

fc2b7da

Merge branch 'master' into add-caching-for-fluxv2-plugin

a7d45dd

revert unintended change

3ae6317

step 19

194f731

Michael's feedback #3

8ffa0cb

Merge branch 'master' into add-caching-for-fluxv2-plugin

e67f7cd

absoludity approved these changes Jul 6, 2021

View reviewed changes

gfichtenholt and others added 11 commits July 6, 2021 00:46

Michael's feedback #4

ab5907c

small change to force CI test run

4e42209

small change to add a name to AvailablePackageDetail response

4b66781

Merge branch 'master' into add-caching-for-fluxv2-plugin

1fd3a74

Michael's feedback #5

9eb7821

add a couple of debug statements to help diagnose CI failures

9aba447

Add --set redis.auth.password=password in CI

bffa721

Fix chart deps version

a8492ac

Merge branch 'master' into add-caching-for-fluxv2-plugin

a7b1491

Merge branch 'master' into add-caching-for-fluxv2-plugin

bddade5

Merge branch 'add-caching-for-fluxv2-plugin' of https://github.com/gf…

c72759b

…ichtenholt/kubeapps into add-caching-for-fluxv2-plugin

gfichtenholt merged commit c079ec1 into vmware-tanzu:master Jul 8, 2021

Kubeapps automation moved this from In progress to Done Jul 8, 2021

gfichtenholt deleted the add-caching-for-fluxv2-plugin branch July 8, 2021 00:12

absoludity mentioned this pull request Jul 8, 2021

Investigate caching layer for kubeapps plug-ins #3032

Open

	for job := range requestChan {
	// The following loop will only terminate when the request channel is closed (and there are no more items)
	for job := range requestChan {

	if request != nil && request.Context != nil && request.Context.Cluster != "" {
	if request != nil && request.GetContext().GetCluster() != "" {

Add caching for fluxv2 plugin (first step toward ##3032) #3044

Add caching for fluxv2 plugin (first step toward ##3032) #3044

Conversation

gfichtenholt commented Jun 25, 2021 • edited

gfichtenholt commented Jun 27, 2021 • edited

absoludity commented Jun 29, 2021

gfichtenholt commented Jun 29, 2021

gfichtenholt commented Jul 1, 2021

absoludity left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfichtenholt Jul 2, 2021 • edited

Choose a reason for hiding this comment

absoludity Jul 6, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfichtenholt Jul 6, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

absoludity left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

absoludity Jul 8, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfichtenholt Jul 6, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfichtenholt commented Jun 25, 2021 •

edited

gfichtenholt commented Jun 27, 2021 •

edited

gfichtenholt Jul 2, 2021 •

edited

absoludity Jul 6, 2021 •

edited

gfichtenholt Jul 6, 2021 •

edited

absoludity Jul 8, 2021 •

edited

gfichtenholt Jul 6, 2021 •

edited