Skip to content

remove dependency of code under cmd on conformance #1073

Open
@nirrozenbaum

Description

@nirrozenbaum

as of today, the code under cmd dir and more specifically inside runner has reference to conformance package.
this includes also the need to copy conformance dir in the Dockerfile.

the only reason for this dependency is the fact that conformance tests need to have a predictable output and therefore a new filter plugin was introduced in the conformance tests.
this is initialized using env var that is set to True and then the scheduler is initialized with a single profile that includes a single (predictable) filter.

see the relevant code here:

if reqHeaderBasedSchedulerForTesting {
scheduler = conformance_epp.NewReqHeaderBasedScheduler()

I think code under cmd shouldn't have any dependency on conformance. the other direction is fine though.
I would like to suggest an alternative approach to remove this dependency (and remove all relevant code like the env var). the suggested change will keep all conformance code encapsulated inside conformance dir, without leaking outside to any other package.

recently some changes were made to epp cmd to split the configuration from main.go and into runner package.
these changes result in a very thin main.go, which can configure plugins via code.

we could have under conformance package this kind of main.go file (e.g., under conformance/cmd) and a Dockerfile that builds an image we may call epp-conformance.
the main.go file would look something like the following:

func main() {
	if err := runner.NewRunner().
		WithSchedulerConfig(schedulerConfig). // conformance config, includes a single profile with a predictable filter
		Run(ctrl.SetupSignalHandler()); err != nil {
		os.Exit(1)
	}
}

obviously, the image that will be built is completely identical to the regular epp, with one difference that we set in main.go the predictable scheduler config.
then we could remove conformance dependency from the regular main.go under epp/cmd and from the Dockerfile.

the main advantage is It's a clean solution for removing the conformance dependency.
the downside is that we need to maintain additional docker image (which should be built automatically, same as epp image is built after every PR merges). However I believe it's justified in this case.

I discussed this with @SinaChavoshi shortly in slack, and he sounds supportive.

I'd be happy to hear more thoughts @kfswain @danehans @robscott @ahg-g @SinaChavoshi

Metadata

Metadata

Assignees

No one assigned

    Labels

    gie-area/conformanceCategorizes an issue or PR as relevant to GIE conformance tests.triage/acceptedIndicates an issue or PR is ready to be actively worked on.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions