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.

Sign up for GitHub

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

Jump to bottom

[Failing Test] : [k8s.io] Sysctls tests failing release-upgrade test suite #64845

Closed
AishSundar opened this issue Jun 6, 2018 · 16 comments
Closed

[Failing Test] : [k8s.io] Sysctls tests failing release-upgrade test suite #64845

AishSundar opened this issue Jun 6, 2018 · 16 comments
Assignees
@sjenning @liggitt @ingvagabund
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. milestone/needs-attention priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/node Categorizes an issue or PR as relevant to SIG Node.
Milestone
v1.11

Comments

@AishSundar
Copy link
Contributor

Failing Jobs

This job tests upgrading master and node, in gce(gci), from 1.10 to master, run parallel tests only.

Error Logs

https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-kubernetes-e2e-gce-new-master-upgrade-cluster-parallel/3694

/kind bug
/priority failing-test
/priority critical-urgent
/milestone v1.11
/assign @sjenning
cc @jberkus @tpepper

Possible breaking change #63717
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. kind/bug Categorizes issue or PR as related to a bug. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. labels Jun 6, 2018
@k8s-ci-robot k8s-ci-robot added this to the v1.11 milestone Jun 6, 2018
@k8s-ci-robot k8s-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jun 6, 2018
@sjenning
Copy link
Contributor

sjenning commented Jun 6, 2018

/sig node
/assign @ingvagabund

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 6, 2018
@AishSundar
Copy link
Contributor Author

/status approved-for-milestone

@sjenning
Copy link
Contributor

sjenning commented Jun 6, 2018 •
edited
Loading

It seems the old tests are running against the new code. Line numbers align with test/e2e/common/sysctl.go before the PR.

@liggitt
Copy link
Member

liggitt commented Jun 6, 2018

@ingvagabund can you open a PR against release-1.10 gating the tests using the alpha annotations from running on 1.11.0-alpha.0 or newer?

@ingvagabund
Copy link
Contributor

This job tests upgrading master and node, in gce(gci), from 1.10 to master, run parallel tests only.

Once again, what is the purpose of the test?

@jberkus
Copy link

jberkus commented Jun 6, 2018

It's an upgrade test. Not sure what else you need to know?

@liggitt
Copy link
Member

liggitt commented Jun 6, 2018

in general, the upgrade tests ensure function as exercised in the previous release doesn't regress. the falls apart when the function being tested is explicitly deprecated/dropped in the next version (like these alpha-level annotations). in those cases, we have to fix the previous release branch's tests with magic knowledge from the future.

@AishSundar
Copy link
Contributor Author

@ingvagabund may be this helps - https://github.com/kubernetes/community/blob/master/contributors/devel/e2e-tests.md#version-skewed-and-upgrade-testing

As part of upgrade testing we ensure (among other things) "A newer version of the whole cluster with older tests (e.g. that a cluster upgraded---master and nodes---to v1.3 still passes v1.2 tests)."

So if I am reading this correctly, "gce-1.10-master-upgrade-cluster-parallel" jobs upgrade the entire cluster from 1.10 -> master and then runs 1.10 tests on it. so if the feature is deprecated in 1.11 then the tests will fail. @krzyzacy @BenTheElder to add if I missed / misquoted something

@krzyzacy
Copy link
Member

krzyzacy commented Jun 6, 2018

I mean if a feature is deprecated, it should handle the deprecation in tests as well. Aka what's the intended behavior if you have feature foo enabled in 1.10 and that's deprecated on 1.11?

@liggitt
Copy link
Member

liggitt commented Jun 6, 2018

what's the intended behavior if you have feature foo enabled in 1.10 and that's deprecated on 1.11?

at some point, deprecated function gets dropped. for example, root proxy paths deprecated in 1.2 and dropped in 1.10 required back-patching 1.9 tests that were gamely ensuring the deprecated function kept working

in this case, the thing that got dropped was an alpha annotation, which can be dropped without a deprecation period. in my opinion, tests exercising alpha features shouldn't be part of upgrade tests

@dims
Copy link
Member

dims commented Jun 6, 2018 •
edited
Loading

@krzyzacy the tests in 1.10 branch is running against master. the master changed sysctl stuff from annotations to fields in master, so it will fail.

@BenTheElder
Copy link
Member

in my opinion, tests exercising alpha features shouldn't be part of upgrade tests?

yes, please 👍

This was referenced Jun 6, 2018
Closed
Merged
@ingvagabund
Copy link
Contributor

@liggitt @AishSundar thanks for the explanation.

k8s-github-robot pushed a commit that referenced this issue Jun 7, 2018
Merge pull request #64852 from dims/skip-tests-as-we-promoted-sysctl-…
2e433e8 
…annotations-to-fields-2

Automatic merge from submit-queue.

Skip Sysctl tests against v1.11.0-alpha.0 and newer

**What this PR does / why we need it**:

1.10 e2e tests get run against 1.11.0+ masters during upgrade tests.
This fails as Sysctl moved from annotations to fields, so we should
check the version and avoid running these tests.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #64845

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
@dims
Copy link
Member

dims commented Jun 7, 2018

we have a green run! https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-kubernetes-e2e-gce-new-master-upgrade-cluster-parallel/3709

@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Issue Needs Attention

@AishSundar @ingvagabund @sjenning @kubernetes/sig-node-misc

Action required: During code freeze, issues in the milestone should be in progress.
If this issue is not being actively worked on, please remove it from the milestone.
If it is being worked on, please add the status/in-progress label so it can be tracked with other in-flight issues.

Note: This issue is marked as priority/critical-urgent, and must be updated every 1 day during code freeze.

Example update:

ACK.  In progress
ETA: DD/MM/YYYY
Risks: Complicated fix required
Issue Labels
  • sig/node: Issue will be escalated to these SIGs if needed.
  • priority/critical-urgent: Never automatically move issue out of a release milestone; continually escalate to contributor and SIG through all available channels.
  • kind/bug: Fixes a bug discovered during the current release.
Help

@liggitt
Copy link
Member

liggitt commented Jun 7, 2018

Sounds good

/close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sjenning sjenning

@liggitt liggitt

@ingvagabund ingvagabund

Labels
kind/bug Categorizes issue or PR as related to a bug. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. milestone/needs-attention priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Milestone
v1.11
Development

Successfully merging a pull request may close this issue.

Skip tests as we promoted sysctl annotations to fields dims/kubernetes
10 participants
@dims @jberkus @sjenning @BenTheElder @liggitt @krzyzacy @ingvagabund @k8s-github-robot @AishSundar @k8s-ci-robot