Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

conformance: skip failing tests #205

Closed
wants to merge 28 commits into from

Conversation

mikemorris
Copy link
Contributor

@mikemorris mikemorris commented Jun 1, 2022

Changes proposed in this PR:

Implement our own test runner for the conformance test suite that has a known list of tests to skip. This substantially changes how we approach patching the base manifests now that we're not running the conformance suite from within a local checkout - we apply the kustomzie patch using the krusty library instead of using the CLI.

Note: our conformance tests are failing on a couple of fronts right now:

  • New assertions were added that require us to return a 404 when a rule doesn’t match our request, either because one isn’t configured or because one failed to attach. We can’t currently program Envoy to return a 404 in these cases and so are returning the default 503.
  • The conformance tests for ReferencePolicy wound up with some different behavior expectations based on discussion with others upstream. We want to skip these tests temporarily until we implement the updated logic.
  • Two tests which create gateways as part of the tests rather than through the base manifests (HTTPRouteListenerHostnameMatching and HTTPRouteDisallowedKind) are currently hanging indefinitely, and so have been temporarily skipped until we can track down the cause of that issue.

How I've tested this PR:

  • Conformance tests are now passing in CI again (with a few skipped currently that need followup triage).

Notes for reviewers

How I expect reviewers to test this PR:

  • Decide if we prefer to install Consul via the latest commit on the main branch of the Helm chart source (catches breaking changes earlier, requires keeping global.imageK8S: "docker.mirror.hashicorp.services/hashicorpdev/consul-k8s-control-plane:latest" in consul-config.yaml) or latest published release from the hashicorp Helm repository (incompatible with specifying latest tag for global.imageK8s)
  • Can we PR something upstream to allow setting maxTimeToConsistency in the suite.go file we're reimplementing instead of relying on a fork?

Checklist:

  • Tests added skipped
  • CHANGELOG entry added

    Run make changelog-entry for guidance in authoring a changelog entry, and
    commit the resulting file, which should have a name matching your PR number.
    Entries should use imperative present tense (e.g. Add support for...)

@mikemorris mikemorris added the pr/no-changelog Skip the CI check that requires a changelog entry label Jun 1, 2022
@mikemorris mikemorris force-pushed the conformance/skip-nonconformant-tests branch from dd6f399 to 415af3f Compare June 1, 2022 20:14
Copy link
Contributor Author

@mikemorris mikemorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[ignore]

@mikemorris mikemorris force-pushed the conformance/skip-nonconformant-tests branch from 313218d to b648987 Compare June 7, 2022 17:43
@mikemorris
Copy link
Contributor Author

Well this is now compiling/linting again after all the restructuring and dependency updates, but seems to be getting stuck spinning up the base manifests resources (due to resource constraints?)

@mikemorris
Copy link
Contributor Author

mikemorris commented Jun 8, 2022

After chatting with @nathancoleman realized that the kustomization wasn't actually being applied (needed to write the output back to disk and pass that path as BaseManifests config, krusty doesn't mutate in place). With that done, a few tests skipped for future triage, and a hopefully short-lived workaround for loading manifests from local filesystem pending kubernetes-sigs/gateway-api#1180, conformance tests are now passing again!

I am planning to pull out a few non-relevant changes that could merge separately (like the ParentRef rename) to make this easier to review before pulling out of draft.

nathancoleman and others added 24 commits June 8, 2022 13:59
to fix strings.Cut arg order
@mikemorris mikemorris force-pushed the conformance/skip-nonconformant-tests branch from 1c8acbd to 6363951 Compare June 8, 2022 17:59
@mikemorris mikemorris changed the title conformance: skip nonconformant tests conformance: skip failing tests Jun 8, 2022
@nathancoleman
Copy link
Member

@mikemorris I'm a little fuzzy on this one. Is it something we still need to merge or should it be closed?

@mikemorris
Copy link
Contributor Author

A substantial chunk of this was implemented in #224, the rest is blocked on kubernetes-sigs/gateway-api#1198 currently, probably makes most sense to close this and open a fresh PR later.

@mikemorris mikemorris closed this Jul 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr/no-changelog Skip the CI check that requires a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants