WIP: Reload CRI-O configurations in particular order

[SD-13]

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #7586

Special notes for your reviewer:

Does this PR introduce a user-facing change?

[openshift-ci]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: SD-13
Once this PR has been reviewed and has the lgtm label, please assign kolyshkin for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

Hi @SD-13. Thanks for your PR.

I'm waiting for a cri-o member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[SD-13]

@kwilczynski I know it needs to be improved a lot and the code is little incomplete, but can you please take a look and let me know whether this is a good way of solving the bug? I feel like there is lots of boilerplate code. Can you please help in pointing out how it needs to be improved? Thanks!

[kwilczynski]

@SD-13, the crux of the problem is definitely with the way how the configuration from multiple sources is applied against the current runtime configuration, especially following a reload request.

However, we shouldn't be parsing anything manually. We have a library that does this for us that we use.

Have a look at how configuration is parsed and then how the runtime configuration object is assembled. Then, look at how this path might differ when the reload is requested. Why is it different?

[SD-13]

Hey @kwilczynski I was having some health issues which caused the delay :)

I took a closer look and now not able to figure out how to update the config with the flagset. Let me try to break in down

However, we shouldn't be parsing anything manually. We have a library that does this for us that we use.

Here did you mean cli?

I tried to follow along and see here a flagset object is being created where command line argument values is being set as Value: ....
Here are we updating the config with the flagset values? Seems like this line tells something else which I don't understand clearly. Could you please explain how the flags are being set here to run the app?
btw, I started navigating from main.go.

Have a look at how configuration is parsed and then how the runtime configuration object is assembled. Then, look at how this path might differ when the reload is requested. Why is it different?

What did you mean by this path?

I am trying to do this and figuring out the first 2 cases -

if cmd args were set:
parse the flags and get the updated config
else if env vars were set:
parse the env vars and get the updated config
else if a file path was provided:
Use that as config
else
Use default config

Please take a look and let me know your thoughts on this. Thanks!

[SD-13]

@kwilczynski @saschagrunert Can you please help by pointing out where the default config is being overridden with the command line arguments. I am new to Go and getting lost while navigating.

[saschagrunert]

@kwilczynski @saschagrunert Can you please help by pointing out where the default config is being overridden with the command line arguments.

Yes, this will happen here:

cri-o/internal/criocli/criocli.go

Line 40 in d705c02

func mergeConfig(config *libconfig.Config, ctx *cli.Context) error {

[github-actions]

A friendly reminder that this PR had no activity for 30 days.