Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promote aggregator e2e test to conformance #63947

Merged
merged 1 commit into from Feb 26, 2019

Conversation

@jennybuckley
Copy link
Contributor

jennybuckley commented May 16, 2018

What this PR does / why we need it:
The aggregation API has been GA since 1.9, and it is currently the only way to extend the kubernetes API that is GA. It needs to be conformance tested so kubernetes users can rely on its behavior.

The test is not flaky according to testgrid

Release note:

New conformance tests added for API Aggregation.

/sig api-machinery
/sig architecture
/area conformance

@AishSundar

This comment has been minimized.

Copy link
Contributor

AishSundar commented May 29, 2018

/milestone v1.11
@tpepper @jberkus as FYI

@jennybuckley

This comment has been minimized.

Copy link
Contributor Author

jennybuckley commented May 29, 2018

/kind feature
/priority important-soon

@jennybuckley

This comment has been minimized.

Copy link
Contributor Author

jennybuckley commented May 29, 2018

rebased

@AishSundar

This comment has been minimized.

Copy link
Contributor

AishSundar commented May 29, 2018

@jennybuckley I had a chance to talk to 1.11 RT about this PR. Considering we will be entering CodeFreeze next Tuesday, 6/5, we should ideally aim for this PR to merge by EOD Thursday 5/31 for this to make into 1.11. We will then have time for a couple of test runs before code freeze and branch cut. thanks.

@jennybuckley

This comment has been minimized.

Copy link
Contributor Author

jennybuckley commented May 29, 2018

/test pull-kubernetes-e2e-kops-aws

@deads2k

This comment has been minimized.

Copy link
Contributor

deads2k commented May 31, 2018

I like the idea, but isn't this one of the flaky tests?

@jennybuckley

This comment has been minimized.

Copy link
Contributor Author

jennybuckley commented May 31, 2018

@deads2k
Yes this test is currently flaky. When I made the PR it was one of our most stable tests. My understanding is that recently introduced networking setup issues on the job which runs the test are causing the flakes.

@AishSundar

This comment has been minimized.

Copy link
Contributor

AishSundar commented May 31, 2018

@deads2k good catch this is indeed the top flake and I reopened #63622 yesterday.

@jennybuckley would you be able to take a look at #63622 and see if it can be resolved first (and soon) before merging this PR into 1.11?

@bgrant0607

This comment has been minimized.

Copy link
Member

bgrant0607 commented May 31, 2018

Description: Ensure that the sample-apiserver code from 1.7 and compiled against 1.7
will work on the current Aggregator/API-Server.
*/
framework.ConformanceIt("Should be able to support the 1.7 Sample API Server using the current Aggregator", func() {

This comment has been minimized.

@bgrant0607

bgrant0607 May 31, 2018

Member

Why is this release-specific?

This comment has been minimized.

@sttts

sttts Jun 6, 2018

Contributor

This tests against an old image based on 1.7 k8s.io/apiserver, i.e. it's a kind of version skew test.

The follow-up question: what are our API guarantees here? Will we support aggregated apiservers built against a 1.7 k8s.io/apiserver forever? If not, how far back do we go?

This comment has been minimized.

@lavalamp

lavalamp Jun 6, 2018

Member

Right. It should likely be a rolling window, and 1.7 is likely too old now. The thing is, what we care about is the version of the k8s.io/apiserver code that is linked against. We haven't thought enough about what the requirements for being part of the control plane are. I can see arguments for:

  • aggregated apiserver can be -2 versions & +1 version from main apiserver
  • aggregated apiserver must be at least main apiserver version (seems impractical but that's more or less where we are for webhooks)

This comment has been minimized.

@sttts

sttts Jun 7, 2018

Contributor

Also note that maximal client and aggregated server skew might add up: client-go +1, aggregated server -2, makes effectively +3 for the client.

@k8s-ci-robot k8s-ci-robot added this to the v1.14 milestone Nov 16, 2018

@spiffxp spiffxp added this to To Triage in cncf-k8s-conformance-wg Jan 23, 2019

@fejta-bot

This comment has been minimized.

Copy link

fejta-bot commented Feb 14, 2019

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@apelisse

This comment has been minimized.

Copy link
Member

apelisse commented Feb 14, 2019

ping :-)

@jennybuckley jennybuckley force-pushed the jennybuckley:aggr-conf-test branch from f7b1954 to 819a0a8 Feb 15, 2019

@jennybuckley jennybuckley force-pushed the jennybuckley:aggr-conf-test branch from 819a0a8 to 9e3814f Feb 15, 2019

@jennybuckley jennybuckley force-pushed the jennybuckley:aggr-conf-test branch from 9e3814f to 8101b86 Feb 15, 2019

@jennybuckley

This comment has been minimized.

Copy link
Contributor Author

jennybuckley commented Feb 15, 2019

/retest

@smarterclayton

This comment has been minimized.

Copy link
Contributor

smarterclayton commented Feb 19, 2019

/approve
/lgtm

Thanks

@k8s-ci-robot k8s-ci-robot added the lgtm label Feb 19, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Feb 19, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jennybuckley, smarterclayton

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@spiffxp

This comment has been minimized.

Copy link
Member

spiffxp commented Feb 25, 2019

/hold cancel

@spiffxp

This comment has been minimized.

Copy link
Member

spiffxp commented Feb 25, 2019

/remove-lifecycle stale

@fejta-bot

This comment has been minimized.

Copy link

fejta-bot commented Feb 25, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit d532d50 into kubernetes:master Feb 26, 2019

17 checks passed

cla/linuxfoundation jennybuckley authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Context retired without replacement.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped
tide In merge pool.
Details

cncf-k8s-conformance-wg automation moved this from To Triage to Done Feb 26, 2019

@bclau

This comment has been minimized.

Copy link
Contributor

bclau commented Feb 27, 2019

Hello,

Please let us (me, @adelina-t, @PatrickLang, @michmike) know when you're promoting new tests to Conformance, so we can make sure that the tests also work on Windows. At the moment, it is not, and I've made a PR addressing this issue: #74666

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Feb 27, 2019

Please let us (me, @adelina-t, @PatrickLang, @michmike) know when you're promoting new tests to Conformance, so we can make sure that the tests also work on Windows. At the moment, it is not, and I've made a PR addressing this issue: #74666

is there a way to trigger a windows conformance run against a prospective PR (e.g. /test pull-...)?

@caesarxuchao

This comment has been minimized.

Copy link
Member

caesarxuchao commented Feb 28, 2019

The test is verifying that the aggregator in the master branch (to be 1.14) works with the 1.10 sample apiserver. The version skew is too big. Have we tried to make the test dynamically select the -2 and -1 version of the sample apiserver, instead of hard coding the image version?

@bgrant0607

This comment has been minimized.

Copy link
Member

bgrant0607 commented Mar 5, 2019

+1 to Chao's point. If this test were to fail, it's unclear that anyone should care.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.