golang: add callback method for http plugin config destruction (fixes #38557)#38597
golang: add callback method for http plugin config destruction (fixes #38557)#38597ggreenway merged 13 commits intoenvoyproxy:mainfrom
Conversation
eca7dd7 to
17bb634
Compare
|
/retest |
17bb634 to
98737f2
Compare
…nvoyproxy#38557) Signed-off-by: Francois JACQUES <fjacques@murex.com>
98737f2 to
410aeec
Compare
Signed-off-by: Francois JACQUES <fjacques@murex.com>
1c0741b to
0d023aa
Compare
Signed-off-by: Francois JACQUES <fjacques@murex.com>
Signed-off-by: Francois JACQUES <fjacques@murex.com>
Signed-off-by: Francois JACQUES <fjacques@murex.com>
Signed-off-by: Francois JACQUES <fjacques@murex.com>
Signed-off-by: Francois JACQUES <fjacques@murex.com>
Signed-off-by: Francois JACQUES <fjacques@murex.com>
Signed-off-by: Francois JACQUES <fjacques@murex.com>
Signed-off-by: Francois JACQUES <fjacques@murex.com>
doujiang24
left a comment
There was a problem hiding this comment.
lgtm, no backward compatibility issues.
Signed-off-by: Francois JACQUES <fjacques@murex.com>
|
@doujiang24 I have this alternative design : https://github.com/hypnoce/envoy/commits/go_destroy_config_2/ |
|
@doujiang24 any through on this PR ? |
|
Thanks @hypnoce I'll take a look at the alternative design this weekend. |
Since the |
|
From an API point of view, I think consistency is a good thing, so design #2 seems to me a better one. Technically both solutions are working. I'd rather go with the second design and do a PR for it unless you prefer the first one. You are the maintainer :) |
|
@doujiang24 here the draft PR for alt design: #38767 |
|
Looks like this needs a main merge |
|
@hypnoce Okay, I prefer to merge the current PR, since the It's good enough to merge after a main merge cc @RyanTheOptimist |
Signed-off-by: François JACQUES <fjacques@murex.com>
|
/retest |
Signed-off-by: François JACQUES <fjacques@murex.com>
ggreenway
left a comment
There was a problem hiding this comment.
Approved by senior extension maintainer. I am approving without review.
…proxy#38597) This enables fine grained control over the lifecycle of golang filter config in sync with C++. Some use cases store states and resources in the config object that needs to be cleaned when config is deleted or renewed. The current design uses a Config interface, to minimise changes and avoid breaking existing code. I have an alternative design that adds a Destroy function in the StreamFilterConfigParser interface instead of introducing an interface. Let me know what you think, given the current go api is not considered stable and breaking change should be acceptable. Fixes envoyproxy#38557 Signed-off-by: François JACQUES <fjacques@murex.com>
…proxy#38597) (#7) Signed-off-by: François JACQUES <fjacques@murex.com> Co-authored-by: François JACQUES <fjacques@murex.com>
…proxy#38597) This enables fine grained control over the lifecycle of golang filter config in sync with C++. Some use cases store states and resources in the config object that needs to be cleaned when config is deleted or renewed. The current design uses a Config interface, to minimise changes and avoid breaking existing code. I have an alternative design that adds a Destroy function in the StreamFilterConfigParser interface instead of introducing an interface. Let me know what you think, given the current go api is not considered stable and breaking change should be acceptable. Fixes envoyproxy#38557 Signed-off-by: François JACQUES <fjacques@murex.com>
…proxy#38597) This enables fine grained control over the lifecycle of golang filter config in sync with C++. Some use cases store states and resources in the config object that needs to be cleaned when config is deleted or renewed. The current design uses a Config interface, to minimise changes and avoid breaking existing code. I have an alternative design that adds a Destroy function in the StreamFilterConfigParser interface instead of introducing an interface. Let me know what you think, given the current go api is not considered stable and breaking change should be acceptable. Fixes envoyproxy#38557 Signed-off-by: François JACQUES <fjacques@murex.com>
Commit Message: [#38557] golang: add callback method for http plugin config destruction
Additional Description:
This enables fine grained control over the lifecycle of golang filter config in sync with C++.
Some use cases store states and resources in the config object that needs to be cleaned when config is deleted or renewed.
The current design uses a Config interface, to minimise changes and avoid breaking existing code.
I have an alternative design that adds a Destroy function in the StreamFilterConfigParser interface instead of introducing an interface. Let me know what you think, given the current go api is not considered stable and breaking change should be acceptable.
Risk Level: Low
Testing: A test in config_test.cc was added to ensure Destroy function is being called on the config instance.
Docs Changes: none
Release Notes:
added http golang filter config destroy callback. When a config gets deleted from envoy, the go plugin calls the
Destroyfunction on the config instance.config should implement the new
github.com/envoyproxy/envoy/contrib/golang/common/go/api.Configinterface, implementing the Destroy function.Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]