issue 4837 - chore: main.go refactor

[mrajagopal]

Proposed changes

The objective of this PR is to give test coverage for main.go as outlined in issue 4837. This includes improving functions by:

• returning the error instead of throwing a fatal error
• Split functions to improve readability and testability

Checklist

Before creating a PR, run through this checklist and mark each as complete.

• [x] I have read the CONTRIBUTING doc
• [ ] I have added tests that prove my fix is effective or that my feature works
• [x] I have checked that all unit tests pass after adding my changes
• [x] I have updated necessary documentation
• [x] I have rebased my branch onto main
• [x] I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

[netlify[bot]]

👷 Deploy request for nginx-kubernetes-ingress pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	2a273f840d755f9a337f5c0480a6a901eaeeced5

[jjngx]

Hi @mrajagopal, I want to ask a more general question about what is the intention behind this PR?

[mrajagopal]

Hi @mrajagopal, I want to ask a more general question about what is the intention behind this PR?

@jjngx , the objective of this PR is to give test coverage for main.go. This particular commit attempts to improve functions by:

• returning the error instead of throwing a fatal error
• Split functions to improve readability and testability

I have updated the PR notes to reflect this.

[jjngx]

An idea that may be worth considering:

We have many functions in the main that call log.Fatal in case of an error. It maybe worth considering to the must prefix (eg. mustParse(), mustXYZ()), check the error and call fatal inside the functions. This way we would communicate to the reader that the func either works or NIC won't start, we would have less error checking in the main.

[mrajagopal]

An idea that may be worth considering:

We have many functions in the main that call log.Fatal in case of an error. It maybe worth considering to the must prefix (eg. mustParse(), mustXYZ()), check the error and call fatal inside the functions. This way we would communicate to the reader that the func either works or NIC won't start, we would have less error checking in the main.

@jjngx , this is a good idea, I shall adopt this where possible.

[mrajagopal]

An idea that may be worth considering: We have many functions in the main that call log.Fatal in case of an error. It maybe worth considering to the must prefix (eg. mustParse(), mustXYZ()), check the error and call fatal inside the functions. This way we would communicate to the reader that the func either works or NIC won't start, we would have less error checking in the main.

@jjngx , this is a good idea, I shall adopt this where possible.

Having reviewed all these functions, changing them to not return an error but log the log fatal within the function would make the unit-tests for these function redundant. Therefore, for the time being we can continue with returning an error.

[jjngx]

An idea that may be worth considering: We have many functions in the main that call log.Fatal in case of an error. It maybe worth considering to the must prefix (eg. mustParse(), mustXYZ()), check the error and call fatal inside the functions. This way we would communicate to the reader that the func either works or NIC won't start, we would have less error checking in the main.

@jjngx , this is a good idea, I shall adopt this where possible.

Having reviewed all these functions, changing them to not return an error but log the log fatal within the function would make the unit-tests for these function redundant. Therefore, for the time being we can continue with returning an error.

Excellent, what's better than reducing complexity by removing or not adding more code 👍🏻