Utility functions and specification of EPP command line flags

[elevran]

What type of PR is this?
/kind feature

What this PR does / why we need it:
Allows definition of command line flags in declarative and centralized manner.
Previously split between cmd/epp/runner/runner.go and pkg/epp/server/runserver.go and handling of deprecation was manual.

Which issue(s) this PR fixes:
Fixes #1932

Does this PR introduce a user-facing change?:

NONE

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[netlify]

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	f4b8637
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/69319671e9fa1b00087d6abb
😎 Deploy Preview	https://deploy-preview-1947--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: elevran
Once this PR has been reviewed and has the lgtm label, please assign kfswain for approval. For more information see the 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

[elevran]

/cc @ahg-g @kfswain @nirrozenbaum for early feedback. See #1932 for some additional context.
Is this reasonable (need and approach)?

If so, follow up commits on this PR (or I can split into smaller for easier review) can

• optionally move options.go to pkg/common and leave only cli_flags.go in pkg/epp?
• remove flag definitions and defaults (assuming no cut-n-paste errors on my end) from runner and runserver
• runner.go to use pass []flags to options.AddFlags instead of creating flags directly
• remove functions that are no longer needed (e.g., checking deprecated in runner, setting defaults from command line in datalayer/metrics, etc.)

[shmuelk]

Why not use generics here and have only one function?

That will also cover integers and other types....

[elevran]

That's certainly possible and would genralize and simplify the code somewhat.
Wanted to keep initial code simpler as mostly looking for feedback on whether this is useful before investing more time.

[elevran]

/retest

[nirrozenbaum]

you need to rebase in order to pass the failing test.

[nirrozenbaum]

Thanks for the draft @elevran 🙏
overall the direction is good. having said that I would change few things:

• IMO no need to create a Flag struct. we should reuse the common and widely used patterns from Kubernetes and CNCF subprojects. the common libraries that I'm aware of are pflag or cobra.
• I'm aware of the fact that the above libraries do not support marking flag as deprecated or setting a replacement. with that said, we should be fine with setting the help message of a flag to [DEPRECATED] .... and add ad-hoc printing to the log. keep in mind deprecation of flags is temp state and that pushing a one liner change to mark flag as deprecated is more or less the same as pushing a one liner to print the deprecated flag if used.
• I don't think we should create something common. each runnable (epp and bbr in this case, could be also the predictor sidecar later on), should have its own options.go file, where we create a struct with its cmd-line flags.
• not sure I got the idea of why we have two files options.go and cli_flags.go. I think we should have only options.go.

to summarize I recommend:

• having only one file options.go, with a struct Options. that struct should include the various cmd-line flags and should include the default values in consts. example from kube-proxy is here.
• assuming we choose to use pflag, options file should have a function AddFlags where the flags are added to pflag. example from kube-proxy is here.
• options should also have a function Validate() err where we validate that mandatory flags were set, otherwise return an error (e.g., either a configFile of configText should be set).
• I think we might have benefits from parsing in main.go and then giving the parsed flags as arg to runner. this way we should be able to use parsed flags also in llm-d scheduler main.go if needed (e.g., if we want to use a dedicated cmd-line flag in llm-d, we can extend epp options.go).

LMKWYT.

[elevran]

to summarize RFC:

• moving from flag to pflag sounds reasonable and can be done orthogonally of any other change.
• at this time, there's no desire to create a Flag struct that declaratively defines the EPP flags.
• instead, we'll follow other k8s projects to define an Options struct (instead of individual variables) with AddFlags(), Validate(), etc.
  This will centralize all flag defintions under cmd/main/runner/options.go (including moving defautls from runserver).