Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ServiceSpec is missing possibility to set annotations #21995

Closed
voroniys opened this issue Mar 9, 2020 · 7 comments · Fixed by #22104
Closed

ServiceSpec is missing possibility to set annotations #21995

voroniys opened this issue Mar 9, 2020 · 7 comments · Fixed by #22104

Comments

@voroniys
Copy link
Contributor

voroniys commented Mar 9, 2020

Describe the feature request
I'd like to have in k8s.io.api.core.v1.ServiceSpec a possibility to add annotations to the service.

Describe alternatives you've considered
ATM there is no possibility to set annotations for the service

[ X] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

Additional context

@richardwxn
Copy link
Contributor

we do support setting serviceannotations, did you get any error when trying to do that or you want to know how it can be done?

@voroniys
Copy link
Contributor Author

At least in the source code I have not found it. Ihave

addonComponents:
  kiali:
    k8s:
      service:
and here it stops, since there is no annotation possible.

If you can tell me the secret trick to do it - you are more than wellcome, but anyway properly added and documented feature would be any way better.

@richardwxn
Copy link
Contributor

we did have that documented: https://istio.io/docs/reference/config/istio.operator.v1alpha1/#KubernetesResourcesSpec. But yes our examples are hidden and hard to find, but here is one if you want to take a look to see how to overlay serviceannotations: https://github.com/istio/istio/blob/master/operator/cmd/mesh/testdata/manifest-generate/input/telemetry_k8s_settings.yaml

@voroniys
Copy link
Contributor Author

I've seen all these, but it is not (properly) implemented. May be we need to change this issue type from feature request to bug.

This is what I feed to istioctl:

spec:
  addonComponents:
    kiali:
      enabled: true
      k8s:
        serviceAnnotations:
          test.annotation: test
        service:
          type: NodePort

At least to me seems to be perfectly according to the documentation. And istioctl accepting it without any messages.
But that is what I've got as result of istioctl manifest generate:

apiVersion: v1
kind: Service
metadata:
  name: kiali
  namespace: istio-system
  labels:
    app: kiali
    release: istio
spec:
  ports:
    - name: http-kiali
      protocol: TCP
      port: 20001
  selector:
    app: kiali

where is my annotation? where is my type NodePort? They are all gone! The generated configuration is not what I have expected to have. My service annotation and service type are ignored by istioct.

@richardwxn
Copy link
Contributor

@voroniys I think this is indeed a bug, I just created PR to add those settings in the templates of kiali. But this should happen only for kiali, did you try other components spec?

@voroniys
Copy link
Contributor Author

It looks like it is global issue. For instance I've tried the following with grafana addon both old and new way:

  values:
    grafana:
      service:
        annotations:
          test.annotation: test
        type: NodePort

and it produces correct result:

apiVersion: v1
kind: Service
metadata:
  name: grafana
  namespace: istio-system
  annotations:
    test.annotation: "test"
  labels:
    app: grafana
    release: istio
spec:
  type: NodePort
  ports:
    - port: 3000
      targetPort: 3000
      protocol: TCP
      name: http
  selector:
    app: grafana

but when I try the new way it is broken:

  addonComponents:
    grafana:
      enabled: true
      k8s:
        serviceAnnotations:
          test.annotation: test
        service:
          type: NodePort

I would expect the result should be the same like previous one via values, but it is not:

apiVersion: v1
kind: Service
metadata:
  name: grafana
  namespace: istio-system
  annotations:
  labels:
    app: grafana
    release: istio
spec:
  type: ClusterIP
  ports:
    - port: 3000
      targetPort: 3000
      protocol: TCP
      name: http
  selector:
    app: grafana

Annotation is lost and service type set to ClusterIP instead of nodeport. So it is not kiali only, it is a general bug.

@richardwxn
Copy link
Contributor

@voroniys yes you are right and thanks for reporting it. It is a bug for all the addon components when using k8s override, I already updated my PR to address it

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

Successfully merging a pull request may close this issue.

3 participants