[Testing Framework] Support per app annotations. #13486

incfly · 2019-04-20T00:05:26Z

This allows generic annotations for each app, such that for the Istio features that requiring annotations, we can test it via custmoize the app specifically. For example
- service annotation is used for secure naming in mesh expansion.
- we now also support turn on and off prober rewrite using annotations.
Currently only implemented for Kubernetes environment yet.
Test case for the test framework itself is also added as kube_test.go

Hopefully we can reduce the number of the Istio installation.

istio-testing · 2019-04-20T00:05:30Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: incfly
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: nmittler

If they are not already assigned, you can assign the PR to them by writing /assign @nmittler in a comment when ready.

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:

pkg/OWNERS

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

golangcibot · 2019-04-20T00:06:59Z

pkg/test/framework/components/apps/kube_test.go

+	"io/ioutil"
+	"testing"
+
+	"istio.io/istio/pilot/test/util"


File is not goimports-ed (from goimports)

golangcibot · 2019-04-20T00:07:00Z

pkg/test/framework/components/apps/kube.go

 	if len(a.appName) == 0 {
-		return nil, fmt.Errorf("service does not contain the 'app' label")
+		return nil, fmt.Errorf("Service does not contain the 'App' label")


error strings should not be capitalized or end with punctuation or a newline (from golint)

golangcibot · 2019-04-20T00:07:00Z

pkg/test/framework/components/apps/kube_test.go

+var (
+	runtimeScheme = runtime.NewScheme()
+	codecs        = serializer.NewCodecFactory(runtimeScheme)
+	deserializer  = codecs.UniversalDeserializer()


deserializer is unused (from varcheck)

golangcibot · 2019-04-20T00:07:00Z

pkg/test/framework/components/apps/kube_test.go

+		wantFilePath := fmt.Sprintf("testdata/%v.yaml", name)
+		golden, err := ioutil.ReadFile(wantFilePath)
+		gotBytes := []byte(got)
+		wantBytes := []byte(golden)


unnecessary conversion (from unconvert)

golangcibot · 2019-04-20T00:07:00Z

pkg/test/framework/components/apps/kube_test.go

+)
+
+var (
+	runtimeScheme = runtime.NewScheme()


U1000: var runtimeScheme is unused (from unused)

golangcibot · 2019-04-20T00:07:01Z

pkg/test/framework/components/apps/kube_test.go

+
+var (
+	runtimeScheme = runtime.NewScheme()
+	codecs        = serializer.NewCodecFactory(runtimeScheme)


U1000: var codecs is unused (from unused)

…eck-annotation

howardjohn

I like the change but it may be better off going into Echo which will deprecate Apps component soon

#13175

howardjohn · 2019-04-22T15:49:48Z

pkg/test/framework/components/apps/apps.go

@@ -65,6 +65,13 @@ type App interface {
 type AppParam struct {
 	Name     string
 	Locality string
+	// ServiceAnnotations specifies the annotations for the deployment.


for the deployment or for the Service?

nmittler

Are native apps completely unaffected?

nmittler · 2019-04-22T15:54:31Z

pkg/test/framework/components/apps/kube.go

-		"serviceAccount":  strconv.FormatBool(d.serviceAccount),
-		"locality":        d.locality,
-	})
+	// TODO(incfly): retire InjectProxy field and use annotation.


Just do it in this PR

incfly · 2019-04-22T17:24:47Z

Thanks for the heads up on echo component updates. I haven't stayed tuned with the native & echo implementation. IIRC, they don't have sidecar injector right? Annotations are consumed by either pilot or sidecar injector. Not sure how will these be reflected in native environment?

Thoughts? @howardjohn @nmittler @ozevren

howardjohn · 2019-04-22T17:27:09Z

Annotations can impact pilot so still useful even without sidecar injector I think

incfly · 2019-04-22T17:31:10Z

In native environment, Pilot is configured thru MCP/galley, right? Does that mean we need to plumb this info to Galley in case of native env?

ozevren · 2019-04-22T17:54:40Z

How does the annotations impact Pilot? The only way I am aware of is endpoints. Is there anything else? If so, the solution seem to be making sure that the endpoint information flows correctly in the native case.

…

On Mon, Apr 22, 2019 at 10:41 AM istio-bot ***@***.***> wrote: @incfly <https://github.com/incfly>: The following tests *failed*, say /retest to rerun them all: Test name Commit Details Rerun command prow/istio-unit-tests.sh 886bc3c <886bc3c> link <https://k8s-gubernator.appspot.com/build/istio-prow/pr-logs/pull/istio_istio/13486/istio-unit-tests/24192> /test istio-unit-tests prow/istio-integ-k8s-tests.sh 886bc3c <886bc3c> link <https://k8s-gubernator.appspot.com/build/istio-prow/pr-logs/pull/istio_istio/13486/istio-integ-k8s-tests/7137> /test istio-integ-k8s-tests prow/istio-integ-local-tests.sh 95b4fe0 <95b4fe0> link <https://k8s-gubernator.appspot.com/build/istio-prow/pr-logs/pull/istio_istio/13486/istio-integ-local-tests/9113> /test istio-integ-local-tests Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/guide/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. I understand the commands that are listed here <https://go.k8s.io/bot-commands>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13486 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AECHYIYJAOO2IZMMLBS2AGLPRX2DXANCNFSM4HHIQNPA> .

nmittler · 2019-04-22T18:23:56Z

@incfly I'm very close to landing the echo changes #13175 ... would you mind holding off on this until then?

incfly · 2019-04-22T18:28:29Z

@nmittler Sure.

@ozevren AFAIK, endpoints and service annotations.

istio/pilot/pkg/serviceregistry/kube/conversion.go

Line 40 in 4074b30

KubeServiceAccountsOnVMAnnotation = "alpha.istio.io/kubernetes-serviceaccounts"

incfly · 2019-04-22T18:28:43Z

/hold until #13175 is merged.

istio-testing · 2019-04-22T19:04:00Z

@incfly: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
prow/istio-integ-local-tests.sh	`95b4fe0`	link	`/test istio-integ-local-tests`
prow/istio-integ-k8s-tests.sh	`95b4fe0`	link	`/test istio-integ-k8s-tests`

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. I understand the commands that are listed here.

incfly · 2019-04-24T01:37:58Z

@nmittler your PR now is merged. Is it ready for us to revive this PR? Or we still need to wait for ollow-up changes on echo/apps component?

nmittler · 2019-04-24T14:34:23Z

@incfly I'm currently working out a couple of issues with Echo which should probably be fixed first. I'm hoping to have it resolved over the next couple of days.

incfly · 2019-04-24T17:59:10Z

@nmittler okay, is there a issue number of pr for me to track?

nmittler · 2019-04-24T22:40:19Z

#13553 and #13599

stale · 2019-05-08T23:23:06Z

This pull request has been automatically marked as stale because it has not had activity in the last 2 weeks. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

nmittler · 2019-05-09T14:28:20Z

@incfly apps has been replaced by Echo. Can we close this in favor of #13945?

Adds support for specifying service and workload annotations for Echo deployments. Replaces istio#13486

Adds support for specifying service and workload annotations for Echo deployments. Replaces #13486

Jianfei Hu added 10 commits April 17, 2019 16:36

add the prow user to scp as well.

b1fb96f

Merge branch 'master' of https://github.com/istio/istio

d6c88a0

initial kube_test.go

389f5b4

refactor to renderTemplate.

a9b2465

WIP about to rename

225b6c6

WIP about to rename

1f82579

complete test case.

65225e3

revert raw_vm.go chagne accideentaly

f989853

add refresh feature.

234eb0e

move u referesh

7069da9

istio-testing requested review from nmittler and ozevren April 20, 2019 00:05

googlebot added the cla: yes label Apr 20, 2019

golangcibot reviewed Apr 20, 2019

View reviewed changes

incfly changed the title ~~[Testing Framework] Support annotations in Apps component~~ [Testing Framework] Support per app annotations. Apr 20, 2019

Jianfei Hu added 2 commits April 19, 2019 17:20

Merge branch 'master' of https://github.com/istio/istio into healthch…

a115854

…eck-annotation

lint fixing

886bc3c

howardjohn reviewed Apr 22, 2019

View reviewed changes

nmittler reviewed Apr 22, 2019

View reviewed changes

address comments and update the template to be fake hub and tag.

c2ad8bf

update doc string.

95b4fe0

stale bot added the stale label May 8, 2019

nmittler mentioned this pull request May 9, 2019

[Test Framework] Support annotations in Echo component #13945

Merged

stale bot removed the stale label May 9, 2019

incfly closed this May 9, 2019

nmittler added a commit to nmittler/istio that referenced this pull request May 13, 2019

[Test Framework] Support annotations in Echo component

2acecfa

Adds support for specifying service and workload annotations for Echo deployments. Replaces istio#13486

istio-testing pushed a commit that referenced this pull request May 13, 2019

[Test Framework] Support annotations in Echo component (#13945)

fb6b2c7

Adds support for specifying service and workload annotations for Echo deployments. Replaces #13486

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Testing Framework] Support per app annotations. #13486

[Testing Framework] Support per app annotations. #13486

incfly commented Apr 20, 2019

istio-testing commented Apr 20, 2019

golangcibot Apr 20, 2019

golangcibot Apr 20, 2019

golangcibot Apr 20, 2019

golangcibot Apr 20, 2019

golangcibot Apr 20, 2019

golangcibot Apr 20, 2019

howardjohn left a comment

howardjohn Apr 22, 2019

incfly Apr 22, 2019

nmittler left a comment

nmittler Apr 22, 2019

incfly Apr 22, 2019

incfly commented Apr 22, 2019

howardjohn commented Apr 22, 2019

incfly commented Apr 22, 2019

ozevren commented Apr 22, 2019 via email

nmittler commented Apr 22, 2019

incfly commented Apr 22, 2019

incfly commented Apr 22, 2019

istio-testing commented Apr 22, 2019

incfly commented Apr 24, 2019

nmittler commented Apr 24, 2019

incfly commented Apr 24, 2019

nmittler commented Apr 24, 2019

stale bot commented May 8, 2019

nmittler commented May 9, 2019

[Testing Framework] Support per app annotations. #13486

[Testing Framework] Support per app annotations. #13486

Conversation

incfly commented Apr 20, 2019

istio-testing commented Apr 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

howardjohn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmittler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

incfly commented Apr 22, 2019

howardjohn commented Apr 22, 2019

incfly commented Apr 22, 2019

ozevren commented Apr 22, 2019 via email

nmittler commented Apr 22, 2019

incfly commented Apr 22, 2019

incfly commented Apr 22, 2019

istio-testing commented Apr 22, 2019

incfly commented Apr 24, 2019

nmittler commented Apr 24, 2019

incfly commented Apr 24, 2019

nmittler commented Apr 24, 2019

stale bot commented May 8, 2019

nmittler commented May 9, 2019