Updating endpoints only does not require previous snapshot #4403
Conversation
github-actions
bot
added
the
keep pr updated
…parate-eds-update
|
Issues linked to changelog: |
|
|
||
| cancel() | ||
| }) | ||
|
|
||
| It("updates Envoy endpoints even if proxy is rejected", func() { | ||
|
|
||
| By("create a deployment and a matching service") |
There was a problem hiding this comment.
I moved it to the BeforeEach. Now each test sets up the appDeployment and appService before the test, and then deletes it after.
| @@ -337,8 +459,14 @@ func (t *mockTranslator) Translate(params plugins.Params, proxy *v1.Proxy) (envo | |||
| } | |||
| } | |||
| } | |||
| if t.currentSnapshot != nil { | |||
There was a problem hiding this comment.
in the code this part is in the syncer. why is it in mock translator here? should it be in level above?
There was a problem hiding this comment.
This is just the test to be able to check the non NilSnapshot case. Otherwise syncEnvoy will use the NilSnapshot instead when it tests Sync (https://github.com/solo-io/gloo/blob/master/projects/gloo/pkg/syncer/envoy_translator_syncer.go#L125). When the mock translator has a snapshot set it will use that and we can verify only the endpoints/clusters were updated.
| endpoints cache.Resources, | ||
| clusters cache.Resources, | ||
| ) cache.Snapshot { | ||
| // TODO: Copy resources and downgrade, maybe maintain hash to not do it too many times |
There was a problem hiding this comment.
Is there an issue link for this? If so, can you include the link here?
There was a problem hiding this comment.
There isn't right now but I can make one. I think I'll remove this comment and just keep the one in NewSnapshotFromResources so we don't have it in two spots.
| }, | ||
| }, | ||
| } | ||
| _, err := upstreamClient.Write(upstream, clients.WriteOpts{Ctx: ctx, OverwriteExisting: true}) |
There was a problem hiding this comment.
Looks like this err is unchecked.
…parate-eds-update
* Merge branch 'separate-eds-update' of github.com:solo-io/gloo into separate-eds-update
* Updating endpoints only does not require previous snapshot (#4403) * Merge branch 'separate-eds-update' of github.com:solo-io/gloo into separate-eds-update * fix changelog * correct dependency bump for solo-kit * Merge refs/heads/v1.6.x into eds-missing-snapshot-fix-backport * update solo-kit * Merge branch 'eds-missing-snapshot-fix-backport' of github.com:solo-io/gloo into eds-missing-snapshot-fix-backport * deleted dup file * generate code * bump k8s-utils * enable ephemeral containers * skipClusterCreation * fix robustness test, remove k8s-utils bump * fix robustness test * port forward * remove focus * fix robustness test flake
Description
If there are user config errors that would normally prevent Gloo from starting up, we still need the control plane to be able to serve EDS updates even when the snapshot cache has been cleared. EDS updates should occur in all scenarios when the pod is running, regardless if there is a previous snapshot.
Context
Need to ensure Gloo pod starts and serves EDS regardless of config errors when the pod is bounced and snapshot cache is cleared. EDS updates should occur in all scenarios when the pod is running, even when the snapshot cache has been cleared (bouncing gloo pod, settings change, etc.)
#4345
Checklist:
make install-go-tools generated-codeto ensure there will be no code diffBOT NOTES:
resolves Ensure Gloo pod starts and serves EDS regardless of config errors #4345