Changes to API for mesh config integration by ostromart · Pull Request #210 · istio/operator

ostromart · 2019-08-17T02:48:20Z

This reflects the first change to align better with mesh config:

removed all config items from API
removed redundant "common" path
removed per-component values overrides. All values overrides now go through single global path.
updated UT and profiles accordingly

richardwxn

Actually I like the idea of removing the common part and move per component values to global values, which makes the api easier to manipulate and understand

README.md

data/profiles/default.yaml

…e-api

richardwxn · 2019-08-19T20:44:35Z

pkg/translate/translate_value_test.go

 )

 func TestValueToProto(t *testing.T) {
+	t.Skip("Disabled while components.common.values is removed.")


Add a TODO here?

I've created istio/istio#16390.

richardwxn · 2019-08-19T20:45:29Z

cmd/mesh/manifest.go

@@ -111,13 +111,6 @@ func genManifests(inFilename string, setOverlayYAML string) (name.ManifestMap, e

 // fetchInstallPackageFromURL downloads installation packages from specified URL.
 func fetchInstallPackageFromURL(mergedICPS *v1alpha2.IstioControlPlaneSpec) error {


so we are removing this functionality for specifying version and install?

for 1.3 only. we will discuss this again in 1.4 timeframe.

ostromart · 2019-08-20T23:28:19Z

/cherrypick release-1.3

istio-testing · 2019-08-20T23:28:21Z

@ostromart: #210 failed to apply on top of branch "release-1.3":

.git/rebase-apply/patch:1559: space before tab in indent.
 	AutoInjection     *AutoInjectionFeatureSpec     `protobuf:"bytes,30,opt,name=auto_injection,json=autoInjection,proto3" json:"auto_injection,omitempty"`
.git/rebase-apply/patch:1560: space before tab in indent.
 	Gateways          *GatewayFeatureSpec           `protobuf:"bytes,31,opt,name=gateways,proto3" json:"gateways,omitempty"`
.git/rebase-apply/patch:1561: space before tab in indent.
 	// Overrides for default global values.yaml.
.git/rebase-apply/patch:1582: space before tab in indent.
 	// Path or name for the profile e.g.
.git/rebase-apply/patch:1583: space before tab in indent.
 	//   minimal (looks in profiles dir for a file called minimal.yaml)
warning: squelched 2 whitespace errors
warning: 7 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	README.md
M	cmd/mesh/manifest.go
M	go.mod
M	go.sum
M	installer.sha
M	pkg/apis/istio/v1alpha2/istiocontrolplane_types.pb.go
M	pkg/translate/translate.go
M	pkg/translate/translate_test.go
A	pkg/vfs/assets.gen.go
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): pkg/vfs/assets.gen.go deleted in HEAD and modified in Changes to API for mesh config integration. Version Changes to API for mesh config integration of pkg/vfs/assets.gen.go left in tree.
Auto-merging pkg/translate/translate_test.go
CONFLICT (content): Merge conflict in pkg/translate/translate_test.go
Auto-merging pkg/translate/translate.go
Auto-merging pkg/apis/istio/v1alpha2/istiocontrolplane_types.pb.go
CONFLICT (content): Merge conflict in pkg/apis/istio/v1alpha2/istiocontrolplane_types.pb.go
Auto-merging installer.sha
CONFLICT (content): Merge conflict in installer.sha
Auto-merging go.sum
Auto-merging go.mod
Auto-merging cmd/mesh/manifest.go
CONFLICT (content): Merge conflict in cmd/mesh/manifest.go
Auto-merging README.md
error: Failed to merge in the changes.
Patch failed at 0001 Changes to API for mesh config integration

Details

In response to this:

/cherrypick release-1.3

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.

* Changes to API for mesh config integration * Review comments, add strategy * Add strategy to k8s settings * Fix unit tests

* Eliminate vfsgen again. (#200) * Fix build commmand for installing go-bindata (#202) * Update makefile for proto related rules (#169) * Update makefile for proto related rules to work for new system * Address comments * Add doc gen * Upload updated pb.go files * Update common * Address comment * Merge two install sections into one (#203) * Merge two install sections into one * Change section title * Update to 2019-08-16 commonfiles (#207) Enable MacOS based container builds. Depends-On: istio/common-files#30 Depends-On: istio/tools#274 * support resource type for k8s mapping. (#206) * support resource type for k8s mapping. * apply comments. * Fix reverse translation issue (#205) * Fix reverse translation issue * Address hpa spec issues * Refactor * Second version of refactor * Address comment * lint * Changes to API for mesh config integration (#210) * Changes to API for mesh config integration * Review comments, add strategy * Add strategy to k8s settings * Fix unit tests * update doc. (#212) * update doc. * apply comments. * update link with recent source. * Clean up values (#214) * remove invalid and duplicated fields. (#217)

Changes to API for mesh config integration

8ee4829

ostromart requested review from ayj, costinm, martonsereg, morvencao, rcernich, richardwxn, sdake and waynz0r as code owners August 17, 2019 02:48

googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Aug 17, 2019

istio-testing added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 17, 2019

richardwxn reviewed Aug 18, 2019

View reviewed changes

morvencao reviewed Aug 18, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

morvencao reviewed Aug 18, 2019

View reviewed changes

data/profiles/default.yaml Show resolved Hide resolved

istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Aug 18, 2019

ostromart added 2 commits August 19, 2019 10:32

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

ded309b

…e-api

Review comments, add strategy

8f116fd

istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Aug 19, 2019

ostromart added 2 commits August 19, 2019 12:13

Add strategy to k8s settings

3db9087

Fix unit tests

7b30c96

richardwxn reviewed Aug 19, 2019

View reviewed changes

ostromart mentioned this pull request Aug 19, 2019

Ensure reverse translate works with new API and Common removed istio/istio#16390

Closed

richardwxn approved these changes Aug 19, 2019

View reviewed changes

istio-testing merged commit e909725 into istio:master Aug 19, 2019

richardwxn mentioned this pull request Aug 20, 2019

Clean up values #214

Merged

ostromart added a commit to ostromart/operator that referenced this pull request Aug 20, 2019

Changes to API for mesh config integration (istio#210)

8d8da19

* Changes to API for mesh config integration * Review comments, add strategy * Add strategy to k8s settings * Fix unit tests

ostromart mentioned this pull request Aug 20, 2019

Cherrypicks from master #221

Merged

ostromart deleted the update-api branch October 8, 2019 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to API for mesh config integration#210

Changes to API for mesh config integration#210
istio-testing merged 5 commits intoistio:masterfrom
ostromart:update-api

ostromart commented Aug 17, 2019

Uh oh!

richardwxn left a comment

Uh oh!

Uh oh!

Uh oh!

richardwxn Aug 19, 2019

Uh oh!

ostromart Aug 19, 2019

Uh oh!

richardwxn Aug 19, 2019

Uh oh!

ostromart Aug 19, 2019

Uh oh!

ostromart commented Aug 20, 2019

Uh oh!

istio-testing commented Aug 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		@@ -111,13 +111,6 @@ func genManifests(inFilename string, setOverlayYAML string) (name.ManifestMap, e

		// fetchInstallPackageFromURL downloads installation packages from specified URL.
		func fetchInstallPackageFromURL(mergedICPS *v1alpha2.IstioControlPlaneSpec) error {

Conversation

ostromart commented Aug 17, 2019

Uh oh!

richardwxn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

richardwxn Aug 19, 2019

Choose a reason for hiding this comment

Uh oh!

ostromart Aug 19, 2019

Choose a reason for hiding this comment

Uh oh!

richardwxn Aug 19, 2019

Choose a reason for hiding this comment

Uh oh!

ostromart Aug 19, 2019

Choose a reason for hiding this comment

Uh oh!

ostromart commented Aug 20, 2019

Uh oh!

istio-testing commented Aug 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants