fix k8s overlays for addons #22104

richardwxn · 2020-03-11T21:51:46Z

fix #21995

update the k8s overlay logic for addonComponents.
update kiali charts to include the service type and annotation options

Notes: the reordering of some fields in the test output is caused by the json merge generates different order with original yaml

howardjohn

I thought this is what the overlays were for?

operator/pkg/apis/istio/v1alpha1/values_types.proto

ostromart · 2020-03-13T00:31:01Z

operator/pkg/translate/translate.go

+
+		var found bool
+		var m interface{}
+		if componentName == name.AddonComponentName {


it would be better to do this inside renderFeatureComponentPathTemplate as a special case template. this kind of fixup if going to be hard to read and maintain.

I can create a helper function for this, would that be better? The reason is that we are not just changing the templates but there is also some logic changed required(due to the limitation of the tpath code we discussed offline)

operator/pkg/translate/translate.go

ostromart · 2020-03-13T00:34:30Z

manifests/istio-telemetry/kiali/templates/service.yaml

@@ -3,10 +3,15 @@ kind: Service
 metadata:
  name: kiali
  namespace: {{ .Release.Namespace }}
+  annotations:


per john's comment this strikes me as the wrong approach - we should not be adding things that are in IOP to values.yaml.

But I think for now we should keep it consistent with other templates, which means that we should have it before we fully remove helm support.

we are quite likely to move the kiali chart out of the repo shortly so this is not likely to be a lasting change. does this pr work without this change? if not we will have to tackle this problem sooner or later.

without this, user can still do overlay via IOP API, they just cannot use the helm values pass through, but we actually can do that for other addons. I think we'd better to remove all of them at once to keep it consistent later

richardwxn · 2020-03-16T18:03:11Z

@ostromart PTAL again

operator/cmd/mesh/manifest-generate_test.go

operator/cmd/mesh/testdata/manifest-generate/input/ingressgateway_k8s_settings.yaml

ostromart · 2020-03-18T18:46:36Z

operator/cmd/mesh/testdata/manifest-generate/input/addon_k8s_override.yaml

+    kiali:
+      enabled: true
+      k8s:
+        serviceAnnotations:


could you please also add something like resources here too?

ostromart · 2020-03-18T19:28:51Z

operator/pkg/translate/translate.go

+// getK8SSpecFromIOP is helper function to get k8s spec from IOP using path
+// 1. if component is not an addonComponent, get the spec directly from IOP
+// 2. otherwise convert the path and the root to point it to the entry of addonComponents map.
+// e.x: path "Components.AddonComponents.K8S.ReplicaCount" and root "Spec" ->


very hard to follow this example, could you rewrite in terms of API inputs and outputs?

istio-testing · 2020-03-18T21:20:50Z

In response to a cherrypick label: #22104 failed to apply on top of branch "release-1.5":

Using index info to reconstruct a base tree...
A	operator/cmd/mesh/testdata/manifest-generate/output/ingressgateway_k8s_settings.golden.yaml
M	operator/pkg/apis/istio/v1alpha1/values_types.proto
Falling back to patching base and 3-way merge...
Auto-merging operator/pkg/apis/istio/v1alpha1/values_types.proto
Auto-merging operator/cmd/mesh/testdata/manifest-generate/output/ingressgateway_k8s_settings.yaml
Applying: fix overlay for k8s
.git/rebase-apply/patch:14342: trailing whitespace.
    
.git/rebase-apply/patch:14350: trailing whitespace.
      
.git/rebase-apply/patch:14395: trailing whitespace.
    
.git/rebase-apply/patch:14402: trailing whitespace.
    
.git/rebase-apply/patch:14447: trailing whitespace.
    
warning: 5 lines add whitespace errors.
Using index info to reconstruct a base tree...
A	operator/cmd/mesh/testdata/manifest-generate/output/all_on.golden-show-in-gh-pull-request.yaml
A	operator/cmd/mesh/testdata/manifest-generate/output/component_hub_tag.golden.yaml
A	operator/cmd/mesh/testdata/manifest-generate/output/flag_output_set_values.golden.yaml
A	operator/cmd/mesh/testdata/manifest-generate/output/flag_set_values.golden.yaml
A	operator/cmd/mesh/testdata/manifest-generate/output/gateways_override_default.golden.yaml
A	operator/cmd/mesh/testdata/manifest-generate/output/helm_values_enablement.golden.yaml
A	operator/cmd/mesh/testdata/manifest-generate/output/prometheus.golden.yaml
M	operator/pkg/apis/istio/v1alpha1/v1alpha1.pb.html
M	operator/pkg/apis/istio/v1alpha1/values_types.pb.go
M	operator/pkg/apis/istio/v1alpha1/values_types.proto
A	operator/pkg/component/component.go
M	operator/pkg/translate/translate.go
M	operator/pkg/vfs/assets.gen.go
Falling back to patching base and 3-way merge...
Auto-merging operator/pkg/vfs/assets.gen.go
Auto-merging operator/pkg/translate/translate.go
Auto-merging operator/pkg/component/component/component.go
Auto-merging operator/pkg/apis/istio/v1alpha1/values_types.proto
Auto-merging operator/pkg/apis/istio/v1alpha1/values_types.pb.go
CONFLICT (content): Merge conflict in operator/pkg/apis/istio/v1alpha1/values_types.pb.go
Auto-merging operator/pkg/apis/istio/v1alpha1/v1alpha1.pb.html
Auto-merging operator/cmd/mesh/testdata/manifest-generate/output/prometheus.yaml
CONFLICT (content): Merge conflict in operator/cmd/mesh/testdata/manifest-generate/output/prometheus.yaml
Auto-merging operator/cmd/mesh/testdata/manifest-generate/output/helm_values_enablement.yaml
CONFLICT (content): Merge conflict in operator/cmd/mesh/testdata/manifest-generate/output/helm_values_enablement.yaml
Auto-merging operator/cmd/mesh/testdata/manifest-generate/output/gateways_override_default.yaml
CONFLICT (content): Merge conflict in operator/cmd/mesh/testdata/manifest-generate/output/gateways_override_default.yaml
CONFLICT (modify/delete): operator/cmd/mesh/testdata/manifest-generate/output/flag_set_values.golden.yaml deleted in HEAD and modified in fix overlay for k8s. Version fix overlay for k8s of operator/cmd/mesh/testdata/manifest-generate/output/flag_set_values.golden.yaml left in tree.
CONFLICT (modify/delete): operator/cmd/mesh/testdata/manifest-generate/output/flag_output_set_values.golden.yaml deleted in HEAD and modified in fix overlay for k8s. Version fix overlay for k8s of operator/cmd/mesh/testdata/manifest-generate/output/flag_output_set_values.golden.yaml left in tree.
CONFLICT (modify/delete): operator/cmd/mesh/testdata/manifest-generate/output/component_hub_tag.golden.yaml deleted in HEAD and modified in fix overlay for k8s. Version fix overlay for k8s of operator/cmd/mesh/testdata/manifest-generate/output/component_hub_tag.golden.yaml left in tree.
Auto-merging operator/cmd/mesh/testdata/manifest-generate/output/all_on.yaml
CONFLICT (content): Merge conflict in operator/cmd/mesh/testdata/manifest-generate/output/all_on.yaml
error: Failed to merge in the changes.
Patch failed at 0002 fix overlay for k8s

* add service to kiali charts * fix overlay for k8s * address comment * address comment and rebase * address comment

richardwxn requested a review from a team as a code owner March 11, 2020 21:51

googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Mar 11, 2020

istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 11, 2020

howardjohn reviewed Mar 11, 2020

View reviewed changes

operator/pkg/apis/istio/v1alpha1/values_types.proto Outdated Show resolved Hide resolved

richardwxn changed the title ~~add service to kiali charts~~ fix k8s overlays for addons Mar 12, 2020

istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Mar 12, 2020

richardwxn force-pushed the fix-kiali-2 branch from b6bd318 to 9a0bea1 Compare March 13, 2020 00:10

istio-testing added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR needs to be rebased before being merged size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 13, 2020

ostromart reviewed Mar 13, 2020

View reviewed changes

operator/pkg/translate/translate.go Show resolved Hide resolved

ostromart reviewed Mar 13, 2020

View reviewed changes

richardwxn force-pushed the fix-kiali-2 branch from 9a0bea1 to 0b9d86f Compare March 15, 2020 21:18

istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Mar 15, 2020

richardwxn force-pushed the fix-kiali-2 branch from 0b9d86f to bfa8077 Compare March 15, 2020 21:25

istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Mar 15, 2020

richardwxn force-pushed the fix-kiali-2 branch from bfa8077 to d4ec5d5 Compare March 15, 2020 21:32

ostromart reviewed Mar 16, 2020

View reviewed changes

operator/cmd/mesh/manifest-generate_test.go Show resolved Hide resolved

ostromart reviewed Mar 16, 2020

View reviewed changes

operator/cmd/mesh/testdata/manifest-generate/input/ingressgateway_k8s_settings.yaml Show resolved Hide resolved

richardwxn force-pushed the fix-kiali-2 branch from d4ec5d5 to 2b253f5 Compare March 18, 2020 08:22

richardwxn added 4 commits March 18, 2020 10:38

add service to kiali charts

4a867f7

fix overlay for k8s

b78e1af

address comment

553faef

address comment and rebase

3b9bf87

richardwxn force-pushed the fix-kiali-2 branch from 2b253f5 to 3b9bf87 Compare March 18, 2020 18:21

ostromart reviewed Mar 18, 2020

View reviewed changes

address comment

2cbe9d2

ostromart approved these changes Mar 18, 2020

View reviewed changes

istio-testing merged commit 84423da into istio:master Mar 18, 2020

richardwxn added the cherrypick/release-1.5 label Mar 18, 2020

richardwxn mentioned this pull request Mar 19, 2020

fix k8s overlays for addons (#22104) #22305

Merged

richardwxn added a commit to richardwxn/istio that referenced this pull request Mar 19, 2020

fix k8s overlays for addons (istio#22104)

a5c3d13

* add service to kiali charts * fix overlay for k8s * address comment * address comment and rebase * address comment

istio-testing pushed a commit that referenced this pull request Mar 19, 2020

fix k8s overlays for addons (#22104) (#22305)

9d31221

* add service to kiali charts * fix overlay for k8s * address comment * address comment and rebase * address comment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix k8s overlays for addons #22104

fix k8s overlays for addons #22104

richardwxn commented Mar 11, 2020 •

edited

Loading

howardjohn left a comment

ostromart Mar 13, 2020

richardwxn Mar 13, 2020

ostromart Mar 13, 2020

richardwxn Mar 13, 2020

ostromart Mar 13, 2020

richardwxn Mar 13, 2020

richardwxn commented Mar 16, 2020

ostromart Mar 18, 2020

richardwxn Mar 18, 2020

ostromart Mar 18, 2020

istio-testing commented Mar 18, 2020

fix k8s overlays for addons #22104

fix k8s overlays for addons #22104

Conversation

richardwxn commented Mar 11, 2020 • edited Loading

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

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

richardwxn commented Mar 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

istio-testing commented Mar 18, 2020

richardwxn commented Mar 11, 2020 •

edited

Loading