Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Gloo Duration type to string #1524

Merged
merged 3 commits into from
Oct 3, 2023
Merged

Change Gloo Duration type to string #1524

merged 3 commits into from
Oct 3, 2023

Conversation

megum1n
Copy link
Contributor

@megum1n megum1n commented Sep 26, 2023

Description

Gloo team generates (result) go structs from protobuf, so not all of them can be used in other applications (such as flagger).
In Gloo Upstream CRD we can see that string is used for timeouts duration configuration instead of object. Because of that, if Upstream's spec.connectionConfig.connectTimeout is set to 2s - flagger throws the following error while trying to create canary upstream from the current upstream:

error creating flagger primary upstream: config upstream test-upstream.test-namespace get query error: json: cannot unmarshal string into Go struct field ConnectionConfig.spec.connectionConfig.connectTimeout of type v1.Duration"

Similar error is also coming from the Gloo admission webhook during canary upstream creation:

Failed to save resource: {"error":{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"admission webhook \"gloo.gloo-test-namespace.svc\" denied the request: resource incompatible with current Gloo snapshot: [1 error occurred:\n\t could not unmarshal raw object: parsing resource from crd spec test-gloo in namespace test-namespace into v1.Upstream: bad Duration: time: missing unit in duration \"2\"\n\n]","details":{"name":"test-upstream","group":"gloo.solo.io","kind":"Upstream","causes":[{"message":"Error 1 error occurred:\n\t could not unmarshal raw object: parsing resource from crd spec test-upstream in namespace test-namespace into v1.Upstream: bad Duration: time: missing unit in duration \"2\"\n\n"}]},"code":400},"messages":["admission webhook \"gloo.gloo-test-namespace.svc\" denied the request: resource incompatible with current Gloo snapshot: [1 error occurred:\n\t could not unmarshal raw object: parsing resource from crd spec test-upstream in namespace test-namespace into v1.Upstream: bad Duration: time: missing unit in duration \"2\"\n\n]"],"isUsedForNotification":false}

Protobuf doc for Duration message is also saying that the string should be used for JSON format.

Changes

  • Changed Gloo Upstream duration type to string
  • Added connectionTimeout configuration to the Gloo canary test

@stefanprodan
Copy link
Member

Looks like the Gloo version used in tests doesn't know about connection timeout:

Upstream in version "v1" cannot be handled as a Upstream: strict decoding error: unknown field "spec.connectionConfig.connectionTimeout"

Signed-off-by: Megum1n <misaka@pantsu.moe>
Signed-off-by: Megum1n <misaka@pantsu.moe>
Signed-off-by: Megum1n <misaka@pantsu.moe>
@megum1n
Copy link
Contributor Author

megum1n commented Oct 3, 2023

@stefanprodan Sorry, I made a typo in the test file. Should be good now.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks @megum1n 🥇

@stefanprodan stefanprodan merged commit 475aff8 into fluxcd:main Oct 3, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants