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

Operator is constantly trying to update OpensearchUser #633

Closed
evheniyt opened this issue Oct 10, 2023 · 4 comments · Fixed by #727
Closed

Operator is constantly trying to update OpensearchUser #633

evheniyt opened this issue Oct 10, 2023 · 4 comments · Fixed by #727

Comments

@evheniyt
Copy link
Contributor

After we had created users with OpensearchUser we noticed that operator is constantly trying to update all created users even when there were no changes.
In operator logs we could see:

operator-controller-manager {"level":"info","ts":"2023-10-10T15:18:07.874Z","msg":"Reconciling OpensearchUser","controller":"opensearchuser","controllerGroup":"opensearch.opster.io","controllerKind":"OpensearchUs
er","OpensearchUser":{"name":"es-consumer","namespace":"opensearch"},"namespace":"opensearch","name":"es-consumer","reconcileID":"4cd41a78-4a6a-4e3e-9ac4-103e0e08980a","user":{"name":"es-consumer","namespace":"op
ensearch"}}                                                                                                                                                                                                         
operator-controller-manager {"level":"info","ts":"2023-10-10T15:18:08.227Z","msg":"user requires update","controller":"opensearchuser","controllerGroup":"opensearch.opster.io","controllerKind":"OpensearchUser","O
pensearchUser":{"name":"es-consumer","namespace":"opensearch"},"namespace":"opensearch","name":"es-consumer","reconcileID":"4cd41a78-4a6a-4e3e-9ac4-103e0e08980a","os_service":"security"}

In OpensearchUser events we could see that OpensearchAPIUpdated counter is constantly increasing:

  Normal  OpensearchAPIUpdated  105s (x26 over 7m5s)  user-controller  user updated in opensearch                                                                                                                   

Unfortunately, from the code it's not clear why exactly that happens, looks like this reflect.DeepEqual(user, userResponse[username]) could be the root cause. I think to understand what is not equal between the current user and the response that operator gets there should be some additional logs (e.g. what exactly is different)

Opensearch version 2.3.0
Operator version 2.4.0

@IIIRADIII
Copy link

Hi @evheniyt .
We will take a look at it, can you provide the configuration you used to create cluster and user, so that we could try and reproduce it?

@evheniyt
Copy link
Contributor Author

@IIIRADIII very basic configuration for cluster and user

apiVersion: opensearch.opster.io/v1
kind: OpenSearchCluster
metadata:
  annotations:
    meta.helm.sh/release-name: test
    meta.helm.sh/release-namespace: opensearch-test
  labels:
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/version: 2.7.0
    helm.sh/chart: test-opensearch-1.0.0
  name: test-opensearch
  namespace: opensearch-test
spec:
  bootstrap:
    affinity: {}
    resources: {}
  confMgmt: {}
  dashboards:
    affinity: {}
    enable: true
    image: docker.io/opensearchproject/opensearch-dashboards:2.7.0
    imagePullPolicy: IfNotPresent
    opensearchCredentialsSecret: {}
    podSecurityContext: {}
    replicas: 1
    resources: {}
    securityContext: {}
    service:
      type: ClusterIP
    tls:
      caSecret: {}
      generate: true
      secret: {}
    version: 2.7.0
  general:
    drainDataNodes: true
    httpPort: 9200
    image: docker.io/opensearchproject/opensearch:2.7.0
    imagePullPolicy: IfNotPresent
    monitoring:
      scrapeInterval: 30s
      tlsConfig: {}
    podSecurityContext: {}
    securityContext: {}
    serviceName: test-opensearch
    setVMMaxMapCount: true
    vendor: Opensearch
    version: 2.7.0
  initHelper:
    imagePullPolicy: IfNotPresent
    resources: {}
    version: 1.27.2-buildx
  nodePools:
  - component: masters
    diskSize: 30Gi
    replicas: 3
    resources:
      limits:
        cpu: 500m
        memory: 2Gi
      requests:
        cpu: 500m
        memory: 2Gi
    roles:
    - master
    - data
  security:
    config:
      adminCredentialsSecret: {}
      adminSecret: {}
      securityConfigSecret: {}
    tls:
      http:
        caSecret: {}
        generate: true
        secret: {}
      transport:
        caSecret: {}
        generate: true
        perNode: true
        secret: {}
apiVersion: opensearch.opster.io/v1
kind: OpensearchUser
metadata:
  name: test-user
  namespace: opensearch-test
spec:
  opensearchCluster:
    name: test-opensearch
  passwordFrom:
    key: test
    name: users-credentials

@oleksii-kalinin
Copy link

Same here with opensearchroles.opensearch.opster.io

apiVersion: opensearch.opster.io/v1
kind: OpensearchRole
metadata:
  name: dashboards-role
spec:
  opensearchCluster:
    name: opensearch-cluster
  clusterPermissions:
  - cluster_monitor
  - cluster_composite_ops
  - manage_point_in_time
  - indices:admin/template*
  - indices:admin/index_template*
  - indices:data/read/scroll*
  indexPermissions:
  - indexPatterns:
    - ".kibana"
    - ".opensearch_dashboards"
    allowedActions:
    - indices_all
  - indexPatterns:
    - ".kibana-6"
    - ".opensearch_dashboards-6"
    allowedActions:
    - indices_all
  - indexPatterns:
    - ".kibana_*"
    - ".opensearch_dashboards_*"
    allowedActions:
    - indices_all
  - indexPatterns:
    - ".tasks"
    allowedActions:
    - indices_all
  - indexPatterns:
    - ".management-beats*"
    allowedActions:
    - indices_all
  - indexPatterns:
    - "*"
    allowedActions:
    - indices:admin/aliases*
Events:
  Type    Reason                Age                  From             Message
  ----    ------                ----                 ----             -------
  Normal  OpensearchAPIUpdated  9s (x12 over 5m14s)  role-controller  role updated in opensearch

@evheniyt
Copy link
Contributor Author

evheniyt commented Feb 13, 2024

Looks like the problem is in different types of OpendistroSecurityRoles and BackendRoles
I have debugged it with "github.com/go-test/deep" and it returns this:

[OpendistroSecurityRoles: <nil slice> != [] BackendRoles: <nil slice> != []]{Password: OpendistroSecurityRoles:[] BackendRoles:[] Attributes:map[k8s-uid:811abea9-daea-48db-9c53-4c6771fca5d6]}

Here is the output for user and userResponse[username] that are being used to compare in reflect.DeepEqual(user, userResponse[username]) here

user: requests.User{Password:"", OpendistroSecurityRoles:[]string(nil), BackendRoles:[]string(nil), Attributes:map[string]string{"k8s-uid":"0e3b50e9-6869-4928-be2b-3e92aee0071b"}}                         
userResponse[username]: requests.User{Password:"", OpendistroSecurityRoles:[]string{}, BackendRoles:[]string{}, Attributes:map[string]string{"k8s-uid":"0e3b50e9-6869-4928-be2b-3e92aee0071b"}}

Because nil slice is not the same as [] slice reflect.DeepEqual(user, userResponse[username]) returns false

prudhvigodithi pushed a commit that referenced this issue Feb 26, 2024
…ler (#727)

### Description
Fixes #633
Fixes #731 

Based on investigations
[here](#633 (comment))
and
[here](#731 (comment))
it was found that a discrepancy between `<nil slice>` and `<[] slice>`
is causing the operator to constantly update User, Role, IndexTemplate,
etc.

Also, the current log level for User reconciles produces too many logs,
so increasing its level may be a good idea here.

Because of the issue with comparing `nil` and empty slices, I have
replaced `reflect.DeepEqual` with `comp.Equal`. Also, added a helper
function that will sort nested json keys in cases when API returns
unsorted keys which is casing comparing to return diff.
### Issues Resolved
Closes #633 
Closes #731

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Yevhenii Tiutiunnyk <[email protected]>
swoehrl-mw pushed a commit to MaibornWolff/opensearch-operator that referenced this issue Apr 25, 2024
…ler (opensearch-project#727)

### Description
Fixes opensearch-project#633
Fixes opensearch-project#731

Based on investigations
[here](opensearch-project#633 (comment))
and
[here](opensearch-project#731 (comment))
it was found that a discrepancy between `<nil slice>` and `<[] slice>`
is causing the operator to constantly update User, Role, IndexTemplate,
etc.

Also, the current log level for User reconciles produces too many logs,
so increasing its level may be a good idea here.

Because of the issue with comparing `nil` and empty slices, I have
replaced `reflect.DeepEqual` with `comp.Equal`. Also, added a helper
function that will sort nested json keys in cases when API returns
unsorted keys which is casing comparing to return diff.
### Issues Resolved
Closes opensearch-project#633
Closes opensearch-project#731

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Yevhenii Tiutiunnyk <[email protected]>
Signed-off-by: Sebastian Woehrl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants