-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
RFC: Deprecation and removal of ComponentConfig #895
Comments
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/lifecycle frozen @christopherhein Let me know if this should fall into 0.7, or we can wait for later as well |
/priority important-longterm |
What is the v0.7 timeframe? I'm working on updating the PR this week and will likely have something Friday or early next week… at least for #891 |
1-2 months, probably, it all depends when 1.19 is release and when the current planned breaking changes will merge |
Perfect, let's aim for that milestone since as we've discussed this will also have breaking changes. Thanks! |
@DirectXMan12 @vincepri looking into at the next phases of this… I'd originally documented moving to use For reference:
The ux would end up being something like:
Expecting that Options an/or the ComponentConfig type set the same defaults we have defined in https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/client/config/config.go#L85-L88 What do you think? Do you think this is/would be useful? |
Just notice there hasn't been much activity and recent deprecations add a bit of confusion (#2165 (comment)). @christopherhein @vincepri is this issue still relevant? |
@errordeveloper Probably not, x-posting from the deprecation PR #2165 (comment)
|
/retitle RFC: Deprecation and removal of ComponentConfig |
I'm afraid, but I have to say that it is now exteremely confusing to anyone reading this issue. |
This is a bit annoying, as we had just finished migrating to these types, as they were what all the examples were showing We were following the "custom" example to embed ComponentConfig into our configuration, and using the file loader to load both our application configuration and to configure controller-runtime in one go. We run about a dozen different controllers and having a consistent way to configure controller-runtime, for internally-developed controllers, as well as integrating open source ones, is extremely attractive for our operations. Now I have egg on my face to all my colleagues (and open source projects) because this was in no way well communicated as being removed on such short notice. |
@terinjokes I have also ended up doing that too, I think it should be actually fairly easy to either just move this code as is into another repo or create a minimalist config loader. Let me know if you want to work together on some kind of solution to this deprecation problem. I find the current API a bit of an overkill for such a small task.
It could be replaced with something simpler. But it's not like it bothers me really, it's just that while figuring out how to use it I had to read what all these fancy functions do, and I wished it was just one simple struct and a loader method instead. |
I can agree the file loader was a bit strange at first, and would be okay with improvements there. The real win is retaining a consistent way to configure controller-runtime across projects. |
There is no graduation issue at the moment, which was part of the problem; if folks would like to make a plan it would be great to update the proposal and discuss async |
Is there any recommended migration paths or examples? I am currently following what this PR from wg-batch has done |
@vincepri What is the process for such a proposal here? |
@vincepri I still would like to get config loading back into controller runtime. How could I begin working on a proposal? |
There is a need to push the original proposal further, and have folks be listed as maintainers long term. What in general hasn't worked well is large features without a clear maintainer, and where the current set of maintainers don't have bandwidth to carry. What also would be ideal is that we restructure the codebase to have the component config as an optional pattern that lives outside the manager altogether. |
Was there an original proposal document? Or do you mean the alpha that got implemented? I haven’t seen the former, if it exists.
I can see this working fairly well, as a lot of this configuration is also exposed as part of the manager’s options. Especially with the improvements we’ve seen with those options in the last two releases. |
Q: The old ComponentConfig was convenient for operator developers because it allowed them or their users to modify all standard operator configuration without having to deal with all the parsing and boilerplate themselves. Now, is there anything that replaces it, or should users simply embed |
This issue tracks the deprecation and removal of ComponentConfig from Controller Runtime.
The text was updated successfully, but these errors were encountered: