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

Use ComponentConfig #107

Closed
luxas opened this issue Nov 14, 2018 · 19 comments
Closed

Use ComponentConfig #107

luxas opened this issue Nov 14, 2018 · 19 comments
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Milestone

Comments

@luxas
Copy link

luxas commented Nov 14, 2018

See my talk on how to do it, but you probably already know ;)
https://sched.co/FuKB
Sample repo: https://github.com/luxas/sample-config

@BenTheElder
Copy link
Member

Yeah! Definitely one of the reasons I used go, we should get around to fixing this :-)
/assign @munnerz

@BenTheElder BenTheElder added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Nov 15, 2018
@neolit123
Copy link
Member

BTW, the codecs in api-machinery are bound to json-iter and do not support strict unmarshalling. json-iter supports it except that it's options are hardcoded.

so if there is a plan for a switch to unmarshaling from codecs instead of plain yaml.UnsmarshalStrict, possibly yaml.UnmarshalStrict has to be moved as a pre-step before decoding.

@BenTheElder
Copy link
Member

oh that is an interesting problem. I'm not overly attached to strict unmarshaling, I actually had an ulterior motive of seeing how well switching went before switching prow's config, but it does seem helpful to actually error on unknown fields..

possibly yaml.UnmarshalStrict has to be moved as a pre-step before decoding.

I'm not sure I follow there, what would that look like roughly?

@luxas
Copy link
Author

luxas commented Nov 17, 2018

@neolit123 I think this should be discussed in the componentconfig KEP threads instead of here, which is more the implementation.

@luxas
Copy link
Author

luxas commented Nov 17, 2018

But yeah, +1 to warn on unknown fields

@neolit123
Copy link
Member

I'm not sure I follow there, what would that look like roughly?

sigs.k8s.io/yaml.UnmarshalStrict(bytes, externalConfigStructInstance) // #1
...
k8s.io/apimachinery/pkg/runtime.DecodeInto(myScheme.Codecs.UniversalDecoder(), bytes, internalConfigStructInstance);

validation happens at #1.

@neolit123
Copy link
Member

commented: kubernetes/community#2354 (comment)
i think that was the KEP thread.

@BenTheElder
Copy link
Member

Hmm, I think we should teach the decoder to handle strict instead then, our homebrew GVK thing works OK if not being the best thing to maintain long term. It seems like everyone will benefit from a strict option as discussed here now by @neolit123 and co. kubernetes/community#2354 (comment)

@luxas
Copy link
Author

luxas commented Nov 19, 2018

@BenTheElder @neolit123 Please redirect discussion to actionable, generic work items of the upcoming k8s.io/component repo you'll utilize. https://docs.google.com/document/d/1nZnzJD9dC0xrtEla2Xa-J6zobbC9oltdHIJ3KKSSIhk/edit
Thanks!

@alejandrox1
Copy link
Contributor

@BenTheElder @munnerz is there any chance that I could help with this? :-)

@BenTheElder
Copy link
Member

currently we aren't configuring things covered by k8s.io/component directly (potentially kubeadm is). we'll adopt it when we configure them.

@BenTheElder BenTheElder added this to the 2019 goals milestone Dec 8, 2018
@luxas
Copy link
Author

luxas commented Dec 8, 2018

@BenTheElder using https://github.com/luxas/sample-config as a reference, you can adopt ComponentConfig meanwhile. Whether you need k8s.io/component or not is to be seen later when it exists but meanwhile it makes sense to adopt the common k8s API machinery I think

@neolit123
Copy link
Member

kind is already doing parts of the componentconfig spec:
https://github.com/kubernetes-sigs/kind/tree/master/pkg/cluster/config

@luxas
Copy link
Author

luxas commented Dec 10, 2018

That's very cool, thanks @fabriziopandini. Do you think there's anything more that needs to be done yet or is this initially done?

@fabriziopandini
Copy link
Member

@luxas after #147 merge there will be internal/external config, conversions, defaulting, scheme that is what required for implementing multi-node.
Let's do a pair code review afterwards to see if something is missing/could be improved

@luxas
Copy link
Author

luxas commented Dec 10, 2018

Let's do a pair code review afterwards to see if something is missing/could be improved

Sounds good @fabriziopandini!

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 27, 2019
@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 27, 2019
@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@neolit123
Copy link
Member

this timeout out it seems.

kind already supports component configuration.
once component-base is ready for wider consumption some of it's utils can be leveraged here if appropriate.

@kubernetes-sigs kubernetes-sigs deleted a comment from fejta-bot Jun 24, 2021
@kubernetes-sigs kubernetes-sigs deleted a comment from fejta-bot Jun 24, 2021
@kubernetes-sigs kubernetes-sigs deleted a comment from fejta-bot Jun 24, 2021
@BenTheElder
Copy link
Member

I'm pretty happy with our component configuration even if it does use a small inline implementation that is not the component-base tooling.

We can revisit that in the future, but I don't see much upside now that we have it, pulling in those tools makes it harder to embed in projects like kinder, which is probably not relevant to most component-base projects but is to kind.

@BenTheElder BenTheElder removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 24, 2021
stg-0 pushed a commit to stg-0/kind that referenced this issue Jun 20, 2023
…_addon

[EOS-11310] [EKS] Cambiar despliegue de coredns como addon
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

No branches or pull requests

7 participants