Migrate serving to istio.io/client-go apis by skaslev · Pull Request #6041 · knative/serving

skaslev · 2019-11-15T11:54:24Z

Proposed Changes

Migrate serving to istio.io/client-go apis.

/lint

Not for merging yet since it depends on knative/pkg#882 and is temporarily pinned to github.com/skaslev/pkg/tree/istio-apis2 for testing.

/hold

knative-prow-robot

@skaslev: 0 warnings.

Details

In response to this:

Proposed Changes

Migrate serving to istio.io/client-go apis.

Fixes #5969 #5936

/lint

Not for merging yet since it depends on knative/pkg#882 and is temporarily pinned to github.com/skaslev/pkg/tree/istio-apis2 for testing.

/hold

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.

knative-prow-robot · 2019-11-15T11:54:39Z

Hi @skaslev. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

skaslev · 2019-11-15T12:03:07Z

One issue I stumbled upon with this migration was that Istio's PortSelector.Name, which Knative is using, got removed recently.

@tcnghia @mattmoor any suggestions how to handle this?

mattmoor · 2019-11-15T14:56:53Z

If I'm not mistaken, we only ever pass an int anyways:

serving/pkg/reconciler/route/resources/ingress.go

Line 179 in 5617874

ServicePort: intstr.FromInt(int(networking.ServicePort(t.Protocol))),

cc @tcnghia WDYT?

vagababov · 2019-11-15T18:08:56Z

I don't think we use names there, no.

Gopkg.toml

tcnghia · 2019-11-15T21:41:29Z

/ok-to-test

skaslev · 2019-11-17T12:28:13Z

/test pull-knative-serving-unit-tests

Gopkg.toml

skaslev · 2019-11-20T09:55:51Z

/retest

skaslev · 2019-11-22T10:55:26Z

/assign @tcnghia

skaslev · 2019-11-26T09:52:55Z

/retest

knative-test-reporter-robot · 2019-11-26T09:59:01Z

The following jobs failed:

Test name	Triggers	Retries
pull-knative-serving-unit-tests		0/3

Failed non-flaky tests preventing automatic retry of pull-knative-serving-unit-tests:

pkg/activator/net.TestMultipleActivators

skaslev · 2019-11-26T10:16:42Z

pkg/activator/net.TestMultipleActivators seems to be flaking since this PR doesn't affect it at all.

/retest

skaslev · 2019-11-26T19:55:49Z

/test pull-knative-serving-go-coverage

knative-metrics-robot · 2019-11-26T20:00:45Z

The following is the coverage report on the affected files.
Say /test pull-knative-serving-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/ingress/resources/virtual_service.go	86.5%	86.1%	-0.4

skaslev · 2019-11-26T20:25:41Z

/hold cancel

mattmoor

/lgtm
/approve

knative-prow-robot · 2019-11-26T20:26:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mattmoor, skaslev

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mattmoor]

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

This patch removes a workaround to avoid empty SubjectAltNames, which is introudce by: knative@d528a69#diff-5adf7106f7f21606140cf8ecdb0bcc92R104 The issue should have been solved by knative#6041.

This patch removes a workaround to avoid empty SubjectAltNames, which is introduced by: knative@d528a69#diff-5adf7106f7f21606140cf8ecdb0bcc92R104 The issue should have been solved by knative#6041.

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 15, 2019

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Nov 15, 2019

knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/API API objects and controllers area/networking area/test-and-release It flags unit/e2e/conformance/perf test issues for product features labels Nov 15, 2019

knative-prow-robot reviewed Nov 15, 2019

View reviewed changes

knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 15, 2019

knative-prow-robot requested review from chizhg, dgerd, mdemirhan and vagababov November 15, 2019 11:54

skaslev mentioned this pull request Nov 15, 2019

Don't rename doc.go since it's expected by k8s code generator istio/client-go#22

Merged

coryrc reviewed Nov 15, 2019

View reviewed changes

Gopkg.toml Outdated Show resolved Hide resolved

knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 15, 2019

markusthoemmes reviewed Nov 20, 2019

View reviewed changes

Gopkg.toml Outdated Show resolved Hide resolved

skaslev mentioned this pull request Nov 20, 2019

Create Go client libraries for Istio CRD resources istio/istio#8772

Closed

3 tasks

knative-prow-robot assigned tcnghia Nov 22, 2019

skaslev mentioned this pull request Nov 26, 2019

Migrate pkg to istio.io/client-go apis knative/pkg#882

Merged

Slavomir Kaslev added 2 commits November 26, 2019 21:44

Update knative.dev/pkg

022572d

Switch serving to istio's client apis

a1c4276

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 26, 2019

mattmoor reviewed Nov 26, 2019

View reviewed changes

knative-prow-robot assigned mattmoor Nov 26, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2019

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 26, 2019

knative-prow-robot merged commit 197028d into knative:master Nov 26, 2019

skaslev deleted the istio-apis2 branch November 28, 2019 13:52

skaslev mentioned this pull request Dec 4, 2019

Migrate serving-operator to isito.io/client-go apis knative/serving-operator#240

Merged

This was referenced Dec 5, 2019

Remove empty SubjectAltNames #6160

Merged

istio-networking failing to update gateway #6157

Closed

Conversation

skaslev commented Nov 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Uh oh!

knative-prow-robot left a comment

Choose a reason for hiding this comment

Proposed Changes

Uh oh!

knative-prow-robot commented Nov 15, 2019

Uh oh!

skaslev commented Nov 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattmoor commented Nov 15, 2019

Uh oh!

vagababov commented Nov 15, 2019

Uh oh!

Uh oh!

tcnghia commented Nov 15, 2019

Uh oh!

skaslev commented Nov 17, 2019

Uh oh!

Uh oh!

skaslev commented Nov 20, 2019

Uh oh!

skaslev commented Nov 22, 2019

Uh oh!

skaslev commented Nov 26, 2019

Uh oh!

knative-test-reporter-robot commented Nov 26, 2019

Uh oh!

skaslev commented Nov 26, 2019

Uh oh!

skaslev commented Nov 26, 2019

Uh oh!

knative-metrics-robot commented Nov 26, 2019

Uh oh!

skaslev commented Nov 26, 2019

Uh oh!

mattmoor left a comment

Choose a reason for hiding this comment

Uh oh!

knative-prow-robot commented Nov 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

skaslev commented Nov 15, 2019 •

edited

Loading

skaslev commented Nov 15, 2019 •

edited

Loading