Skip to content

Bug 1826230: Rebase 1.18#84

Merged
openshift-merge-robot merged 21 commits intoopenshift:masterfrom
stlaz:rebase_1.18_beta.2
Apr 21, 2020
Merged

Bug 1826230: Rebase 1.18#84
openshift-merge-robot merged 21 commits intoopenshift:masterfrom
stlaz:rebase_1.18_beta.2

Conversation

@stlaz
Copy link
Copy Markdown
Contributor

@stlaz stlaz commented Mar 16, 2020

This seems to compile and pass most unit tests.

There are a bunch of unit tests that I think might be failing for me locally since I'm using golang 1.14 and I suspect they added some new checks/improved the old ones.

Let's throw it at CI to see what it thinks about it

@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Mar 16, 2020

/hold
Includes non-mergable commits and there's probably quite a lot of commits that could be squashed

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 16, 2020
@stlaz stlaz force-pushed the rebase_1.18_beta.2 branch from f4b36c0 to 624e19a Compare March 16, 2020 12:10
@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Mar 16, 2020

/retest

@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Mar 16, 2020

/retest
flakes

@stlaz stlaz force-pushed the rebase_1.18_beta.2 branch from 624e19a to 46cbfef Compare March 17, 2020 13:38
@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Mar 18, 2020

/retest
the origin test fix merged, others look like flakes; serial tests exceeded AWS limits

@stlaz stlaz changed the title [wip] Rebase 1.18 beta.2 Rebase 1.18 beta.2 Mar 19, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 19, 2020
@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Mar 23, 2020

/retest
the parallel DC tests pass locally and there does not seem to be anything unusual in logs that would suggest an error caused by this PR.

On the other hand, 2 out of 3 OCM pods only seem to log

E0318 09:06:28.140231       1 authentication.go:104] Unable to authenticate the request due to an error: [invalid bearer token, square/go-jose: error in cryptographic primitive, token lookup failed]

I am not sure why that would be happening unless something performs tests with some kind of miscreant tokens.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 25, 2020
@stlaz stlaz force-pushed the rebase_1.18_beta.2 branch from 46cbfef to 72761a8 Compare March 30, 2020 09:08
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2020
@stlaz stlaz changed the title Rebase 1.18 beta.2 Rebase 1.18 Mar 30, 2020
@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Mar 30, 2020

bumped kube to 1.18

@stlaz stlaz force-pushed the rebase_1.18_beta.2 branch 2 times, most recently from 3ffd44f to 6b00e46 Compare April 3, 2020 11:08
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 3, 2020
@stlaz stlaz force-pushed the rebase_1.18_beta.2 branch from 6b00e46 to b8af519 Compare April 3, 2020 11:20
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 3, 2020
@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Apr 3, 2020

/hold cancel
now free of my repo references

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 3, 2020
Comment thread go.mod
k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.17.3
k8s.io/metrics => k8s.io/metrics v0.17.3
k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.17.3
k8s.io/api => k8s.io/api v0.18.0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wondering why library-go doesn't have to specify replace for k8s repositories?

https://github.com/openshift/library-go/blob/master/go.mod

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it does not depend on k8s.io/kubernetes which seems to raise this requirement, or at least it did when I was trying to remove the replaces

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh, I think you are right.

Comment thread go.mod
k8s.io/kubectl v0.17.3
k8s.io/kubernetes v1.17.3
k8s.io/kube-aggregator v0.18.0
k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread go.mod Outdated

func (r *REST) getLogs(podNamespace, podName string, logOpts *corev1.PodLogOptions) (runtime.Object, error) {
func (r *REST) getLogs(ctx context.Context, podNamespace, podName string, logOpts *corev1.PodLogOptions) (runtime.Object, error) {
logRequest := r.podClient.Pods(podNamespace).GetLogs(podName, logOpts)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fun, it doesn't want ctx ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It only constructs the request, but does no .Do() it, that's because of the streaming below

Comment thread pkg/apps/apiserver/registry/deploylog/wait.go Outdated
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

11 similar comments
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Apr 20, 2020

/hold
e2e-aws-serial got broken by ingress, let me fix it

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 20, 2020
@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Apr 21, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 21, 2020
@stlaz
Copy link
Copy Markdown
Contributor Author

stlaz commented Apr 21, 2020

/retest

@stlaz stlaz changed the title Rebase 1.18 Bug 1826230: Rebase 1.18 Apr 21, 2020
@openshift-ci-robot openshift-ci-robot added bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Apr 21, 2020
@openshift-ci-robot
Copy link
Copy Markdown

@stlaz: This pull request references Bugzilla bug 1826230, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.5.0) matches configured target release for branch (4.5.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

Bug 1826230: Rebase 1.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 7535fa2 into openshift:master Apr 21, 2020
@openshift-ci-robot
Copy link
Copy Markdown

@stlaz: All pull requests linked via external trackers have merged: openshift/openshift-apiserver#84. Bugzilla bug 1826230 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1826230: Rebase 1.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Comment thread images/Dockerfile
@@ -1,9 +1,9 @@
FROM registry.svc.ci.openshift.org/openshift/release:golang-1.12 AS builder
FROM registry.svc.ci.openshift.org/openshift/release:golang-1.13 AS builder
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread images/Dockerfile.rhel
@@ -1,9 +1,9 @@
FROM registry.svc.ci.openshift.org/ocp/builder:golang-1.12 AS builder
FROM registry.svc.ci.openshift.org/ocp/builder:golang-1.13 AS builder
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants