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

Yamls are wrong #258

Closed
felix-dpg opened this issue Oct 20, 2021 · 8 comments · Fixed by #256
Closed

Yamls are wrong #258

felix-dpg opened this issue Oct 20, 2021 · 8 comments · Fixed by #256

Comments

@felix-dpg
Copy link

Summary

Yamls are defined in a wrong way

Problem

In the docs, the operator are applied using this URL:

https://docs.mattermost.com/install/install-kubernetes.html

$ kubectl apply -n mattermost-operator -f https://raw.githubusercontent.com/mattermost/mattermost-operator/master/docs/mattermost-operator/mattermost-operator.yaml

The mattermost-operator.yaml define a service account that is not namespaced:

---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: mattermost-operator
---

but the clusterrolebinding assume that the SA is in the namespace mattermost-operator

---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: mattermost-operator
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: mattermost-operator
subjects:
- kind: ServiceAccount
  name: mattermost-operator
  namespace: mattermost-operator
---

Also the deployment and the service does not have a namespace defined, but use the SA that suppose to be in mattermost-operator namespace.

Deployment has serviceAccountName: mattermost-operator

Possible fixes

apiVersion: v1
kind: ServiceAccount
metadata:
  name: mattermost-operator
  namespace: mattermost-operator

---

apiVersion: v1
kind: Service
metadata:
  creationTimestamp: null
  labels:
    app: mattermost-operator
    name: mattermost-operator
  name: mattermost-operator
  namespace: mattermost-operator
spec:
  ports:
  - name: metrics
    port: 8383
    protocol: TCP
    targetPort: metrics
  selector:
    name: mattermost-operator
  type: ClusterIP
status:
  loadBalancer: {}
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: mattermost-operator
  namespace: mattermost-operator
spec:
  replicas: 1
  selector:
    matchLabels:
      name: mattermost-operator
  template:
    metadata:
      labels:
        name: mattermost-operator
    spec:
      containers:
      - args:
        - --enable-leader-election
        - --metrics-addr=0.0.0.0:8383
        command:
        - /mattermost-operator
        env:
        - name: MAX_RECONCILING_INSTALLATIONS
          value: "20"
        - name: REQUEUE_ON_LIMIT_DELAY
          value: 20s
        image: mattermost/mattermost-operator:v1.15.0
        imagePullPolicy: IfNotPresent
        name: mattermost-operator
        ports:
        - containerPort: 8383
          name: metrics
      serviceAccountName: mattermost-operator
@Szymongib
Copy link
Contributor

Hey, thanks for reporting the issue.

The namespaces are populated thanks to -n mattermost-operator. I think there used to be some idea to make namespace configurable that is why the metadata.namespace was not set, but indeed it does not make much sense due to the Cluster Rolebinding subject.

I will fix it with the upcoming release.

@felix-dpg
Copy link
Author

Yes, is not make sense because there are resource that you cant apply the namespace if you follow the way in that you apply the mattermost yamls.

@felix-dpg
Copy link
Author

felix-dpg commented Oct 20, 2021

Hey:

Also the yamls of CRD are...wrong??

Problem:

In the docs say that learn more about CRD here:

https://github.com/mattermost/mattermost-operator/blob/master/config/crd/bases/installation.mattermost.com_mattermosts.yaml

In the yaml doc CRD, there is a different definition for ingress option, also in this same doc say:

description: 'IngressAnnotations defines annotations passed to the Ingress associated with Mattermost. Deprecated: Use Spec.Ingress.Annotations.'

But in the main page of your k8s installation the file that contain CRDs is different, and of course, the ingress definition is different: Here, need to be spec.ingressAnnotations instead of Spec.Ingress.Annotations.

https://raw.githubusercontent.com/mattermost/mattermost-operator/master/docs/mattermost-operator/mattermost-operator.yaml

              ingressAnnotations:
                additionalProperties:
                  type: string
                type: object
              ingressName:
                description: IngressName defines the name to be used when creating
                  the ingress rules
                type: string

Other question is that why mattermost does not give us the possibility defining Kind: Mattermost to select the servicetype, NodePort for example, instead of the need to create the services by ourself.

@Szymongib
Copy link
Contributor

The CRDs here https://raw.githubusercontent.com/mattermost/mattermost-operator/master/docs/mattermost-operator/mattermost-operator.yaml are aligned with the latest released version while those https://github.com/mattermost/mattermost-operator/blob/master/config/crd/bases/installation.mattermost.com_mattermosts.yaml contain changes from master.

When we do the release the whole installation file is generated from other files so that is when they will be updated.

@felix-dpg
Copy link
Author

So, why not allign the CRD docs with the main docs if at the end is the release that we will apply?

@Szymongib
Copy link
Contributor

@felix-dpg
Copy link
Author

felix-dpg commented Oct 21, 2021

@Szymongib
Copy link
Contributor

Yeah, as I mentioned it happens during the release as the old Operator would not understand changes in the CRD.

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

Successfully merging a pull request may close this issue.

2 participants