8000 THREESCALE-8998 - Allow defining zone topology spread constraints for… by valerymo · Pull Request #830 · 3scale/3scale-operator · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

THREESCALE-8998 - Allow defining zone topology spread constraints for… #830

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 12, 2023
Merged

THREESCALE-8998 - Allow defining zone topology spread constraints for… #830

merged 1 commit into from
Jun 12, 2023

Conversation

valerymo
Copy link
Contributor
@valerymo valerymo commented May 21, 2023 •
edited
Loading

WHAT

Jira: https://issues.redhat.com/browse/THREESCALE-8998
Allow defining zone topology spread constraints for components via APIManager CR

Adding optional TopologySpreadConstraints definition for following 3scale pods:

Component Fresh install adds TopologySpreadConstraints Reconcile TopologySpreadConstraints
apicast-production [v] [v]
apicast-staging [v] [v]
backend-cron [v] [v]
backend-listener [v] [v]
backend-worker [v] [v]
backend-redis [v] [v]
system-app [v] [v]
system-sidekiq [v] [v]
system-searchd [v] [v]
system-memcache [v] [v]
system-mysql [v] [v]
system-postgresql [v] [v]
system-redis [v] [v]
zync [v] [v]
zync-database [v] [v]
zync-que [v] [v]

Validation

*Install 3scale operator* 
$ cd 3scale-operator
$ make install

$ export NAMESPACE=3scale-test
$ oc new-project $NAMESPACE
$ make download
$ make run

Secret and CR to be installed on separate terminal window

ApiManager CR example

apiVersion: apps.3scale.net/v1alpha1
kind: APIManager
metadata:
    name: example-apimanager
spec:
    wildcardDomain: api.vmogilev01.0nno.s1.devshift.org
    resourceRequirementsEnabled: false
    apicast:
        stagingSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management
        productionSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management
    backend:
        listenerSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management
        cronSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management
        workerSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management
        redisTopologySpreadConstraints:
        - maxSkew: 1
          topologyKey: topology.kubernetes.io/zone
          whenUnsatisfiable: ScheduleAnyway
          labelSelector:
            matchLabels:
              app: 3scale-api-management        
    system:
        fileStorage:
          simpleStorageService:
            configurationSecretRef:
              name: s3-credentials
        appSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management        
        sidekiqSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management        
        searchdSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management        
        memcachedTopologySpreadConstraints:
        - maxSkew: 1
          topologyKey: topology.kubernetes.io/zone
          whenUnsatisfiable: ScheduleAnyway
          labelSelector:
            matchLabels:
              app: 3scale-api-management        
        redisTopologySpreadConstraints:
         - maxSkew: 1
           topologyKey: topology.kubernetes.io/zone
           whenUnsatisfiable: ScheduleAnyway
           labelSelector:
             matchLabels:
               app: 3scale-api-management              
        database:
          postgresql:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management
    zync:
        appSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management        
        queSpec:
            topologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management        
        databaseTopologySpreadConstraints:
            - maxSkew: 1
              topologyKey: topology.kubernetes.io/zone
              whenUnsatisfiable: ScheduleAnyway
              labelSelector:
                matchLabels:
                  app: 3scale-api-management

S3 secret example

kind: Secret
apiVersion: v1
metadata: 
  name: s3-credentials
data: 
  AWS_ACCESS_KEY_ID: QUtXXXXXXXXXXXX=
  AWS_SECRET_ACCESS_KEY: bmZl=
  AWS_BUCKET: dGV=
  AWS_REGION: dXMt==
type: Opaque

Check deploymentConfig(s)
Command:

kubectl get deploymentconfigs -o name | xargs -I {} bash -c "echo -n '{} -> topologySpreadConstraints: '; kubectl get {} -o jsonpath='{.spec.template.spec.topologySpreadConstraints}' | yq e -P"

Expected result:

deploymentconfig.apps.openshift.io/apicast-production -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/apicast-staging -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/backend-cron -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/backend-listener -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/backend-redis -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/backend-worker -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/system-app -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/system-memcache -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/system-postgresql -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/system-redis -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/system-searchd -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/system-sidekiq -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/zync -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/zync-database -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/zync-que -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway

Check Update/reconcile flow

oc apply -f apimanagerCR_CheckUpdate.yaml
  • Check DCs
    Command:
$ oc describe dc apicast-staging |grep -i topol
  Topology Spread Constraints:		topology.kubernetes.io/zone:ScheduleAnyway when max skew 1 is exceeded for selector app=3scale-api-management,threescale_component=apicast

Expected result - added threescale_component label:

$ kubectl get deploymentconfigs -o name | xargs -I {} bash -c "echo -n '{} -> topologySpreadConstraints: '; kubectl get {} -o jsonpath='{.spec.template.spec.topologySpreadConstraints}' | yq e -P"
deploymentconfig.apps.openshift.io/apicast-production -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
      threescale_component: apicast
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/apicast-staging -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
      threescale_component: apicast
  maxSkew: 1
  topologyKey: topology.kubernetes.io/zone
  whenUnsatisfiable: ScheduleAnyway
deploymentconfig.apps.openshift.io/backend-cron -> topologySpreadConstraints: - labelSelector:
    matchLabels:
      app: 3scale-api-management
      threescale_component: backend
  maxSkew: 1
.......

** Check pods.
Expected - all pods are updated.
Notes. Checking current issue - with apicast-production and system-app

$ oc get dc
NAME                 REVISION   DESIRED   CURRENT   TRIGGERED BY
apicast-production   2          1         0         config,image(amp-apicast:2.14)
apicast-staging      2          1         1         config,image(amp-apicast:2.14)
backend-cron         2          1         1         config,image(amp-backend:2.14)
backend-listener     2          1         1         config,image(amp-backend:2.14)
backend-redis        2          1         1         config,image(backend-redis:2.14)
backend-worker       2          1         1         config,image(amp-backend:2.14)
system-app           2          1         0         config,image(amp-system:2.14)
system-memcache      2          1         1         config,image(system-memcached:2.14)
system-postgresql    5          1         1         config,image(system-postgresql:2.14)
system-redis         2          1         1         config,image(system-redis:2.14)
system-searchd       2          1         1         config,image(system-searchd:2.14)
system-sidekiq       2          1         0         config,image(amp-system:2.14)
zync                 2          1         1         config,image(amp-zync:2.14)
zync-database        2          1         1         config,image(zync-database-postgresql:2.14)
zync-que             2          1         1         config,image(amp-zync:2.14)

@valerymo valerymo requested a review from a team as a code owner May 21, 2023 13:24
@valerymo valerymo force-pushed the vmo_THREESCALE-8998 branch 7 times, most recently from d6314c3 to 31f4531 Compare May 23, 2023 08:10
@valerymo valerymo changed the title [WIP] THREESCALE-8998 - Allow defining zone topology spread constraints for… THREESCALE-8998 - Allow defining zone topology spread constraints for… May 23, 2023
@eguzki
Copy link
Member
eguzki commented May 29, 2023 •
edited
Loading

Looking good code wise

Notes. Checking current issue - with apicast-production and system-app

Can you give update on that?

eguzki
eguzki reviewed May 29, 2023
View reviewed changes
@eguzki
Copy link
Member
eguzki commented May 29, 2023

missing doc

@valerymo
Copy link
Contributor Author
valerymo commented May 30, 2023 •
edited
Loading

regarding -
"Notes. Checking current issue - with apicast-production and system-app
Can you give update on that? "

It's working now. Problem was that S3 bucket definition was missed in apimanager CR for system. Now it looks ok.

apicast-production-1-l9zjn    1/1     Running     0               4m42s

system-app-1-deploy           0/1     Completed   0               4m46s
system-app-1-hook-post        0/1     Completed   0               55s
system-app-1-hook-pre         0/1     Completed   0               4m44s
system-app-1-r5jcf            3/3     Running     0               2m10s

@valerymo
Copy link
Contributor Author

regarding "missing doc"
@eguzki , could you please help - what I need to do, to document it - where/example? Thank you for help

@eguzki
Copy link
Member
eguzki commented May 30, 2023

regarding "missing doc" @eguzki , could you please help - what I need to do, to document it - where/example? Thank you for help

APIMAnager ref doc needs to be updated

It would also be nice to add an example in the operator's user guide

@valerymo valerymo force-pushed the vmo_THREESCALE-8998 branch from 4537a7d to 7a6c646 Compare June 1, 2023 09:38
@valerymo
Copy link
Contributor Author
valerymo commented Jun 1, 2023 •
edited
Loading

regarding "missing doc" @eguzki , could you please help - what I need to do, to document it - where/example? Thank you for help

APIMAnager ref doc needs to be updated

It would also be nice to add an example in the operator's user guide

@eguzki , it's done - Documentation (ref doc and user guide) updated for both PriorityClassName and TopologySpreadConstraints

@eguzki
Copy link
Member
eguzki commented Jun 1, 2023

/test test-e2e

@valerymo
Copy link
Contributor Author
valerymo commented Jun 1, 2023 •
edited
Loading

all DCs and pods are working now (after adding fileStorage to CR + bucket right definition in secret).
TODO: run 3scale (this branch) with RHOAM + e2e (local); check e2e in the PR

@valerymo
Copy link
Contributor Author
valerymo commented Jun 4, 2023

/test test-e2e

eguzki
eguzki requested changes Jun 6, 2023
View reviewed changes
Copy link
Member
@eguzki eguzki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Almost there. Few nitpicks and should be ready to be merged.

@valerymo valerymo force-pushed the vmo_THREESCALE-8998 branch from bc4e63c to 0a25584 Compare June 6, 2023 12:25
@valerymo
Copy link
Contributor Author
valerymo commented Jun 6, 2023

Hi @eguzki , thank you for Review and comments. Hope I addressed it. It's in separate/last commit.
Regarding your request to check PR with Rhoam - Could you please help. CR created by Rhoam looks differ, it uses affinity. Attached
apimanager-3scale.yaml.txt

eguzki
eguzki requested changes Jun 6, 2023
View reviewed changes
doc/apimanager-reference.md Outdated
@@ -140,6 +140,8 @@ One APIManager custom resource per project is allowed.
| HTTPSProxy | `httpsProxy` | string | No | N/A | Specifies a HTTP(S) Proxy to be used for connecting to HTTPS services. Authentication is not supported. Format is: `<scheme>://<host>:<port>` (see [docs](https://github.com/3scale/APIcast/blob/master/doc/parameters.md#https_proxy-https_proxy)) |
| NoProxy | `noProxy` | string | No | N/A | Specifies a comma-separated list of hostnames and domain names for which the requests should not be proxied. Setting to a single `*` character, which matches all hosts, effectively 8000 disables the proxy (see [docs](https://github.com/3scale/APIcast/blob/master/doc/parameters.md#no_proxy-no_proxy)) |
| ServiceCacheSize | `serviceCacheSize` | int | No | N/A | Specifies the number of services that APICast can store in the internal cache (see [docs](https://github.com/3scale/APIcast/blob/master/doc/parameters.md#apicast_service_cache_size)) |
| PriorityClassName | `priorityClassName` | string | No | N/A | Specifies the Pod priority (see [docs](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#podspec-v1-core:~:text=higher%20the%20priority.-,priorityClassName,-string)) |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I see from your link:

Screenshot 2023-06-06 at 15-39-20 Kubernetes API Reference Docs

As description, I would say what its in the podspec-v1-core, i.e.

If specified, indicates the pod's priority. "system-node-critical" and "system-cluster-critical" are two special keywords which indicate the highest priorities with the former being the highest priority. Any other name must be defined by creating a PriorityClass object with that name. If not specified, the pod priority will be default or zero if there is no default.

As link, I would include the k8s official doc about pod priority

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I tried this link - https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#podspec-v1-core:~:text=higher%20the%20priority.-,priorityClassName,-string -- It show less than second screen as you see (beginning of the doc),but after this it jump to following (or near following in podspec-v1-core, And I can see this image:
Screenshot from 2023-06-06 18-21-06

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But ok, I will replace it as you recommended to pod priority

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Doc updated. Thank you for comment.

@valerymo valerymo force-pushed the vmo_THREESCALE-8998 branch from 0a25584 to 8e58925 Compare June 6, 2023 15:31
eguzki
eguzki approved these changes Jun 7, 2023
View reviewed changes
Copy link
Member
@eguzki eguzki left a comment

Choose a reason for hiding th A3E2 is comment

The reason will be displayed to describe this comment to others. Learn more.

another good job @valerymo 🎖️

@valerymo
Copy link
Contributor Author
valerymo commented Jun 7, 2023

@eguzki Thanks very much for your help and comments !

@eguzki
Copy link
Member
eguzki commented Jun 7, 2023

@valerymo there are conflicts with the recently merged #832

Let me know if you need help to resolve the conflict. I do not foresee a hard conflict. Features do not conflict, just some same files updated from two different PRs

@valerymo
Copy link
Contributor Author
valerymo commented Jun 8, 2023

@eguzki , sorry, I just found this comment that need to do rebase. I tried it, and it's not just these 2 files, but a lot of files require to take my changes. So I will do it carefully on Sunday, hope it's ok

@valerymo valerymo force-pushed the vmo_THREESCALE-8998 branch from 8e58925 to 1a8d730 Compare June 11, 2023 04:54
@valerymo valerymo force-pushed the vmo_THREESCALE-8998 branch from 1a8d730 to b93e37a Compare June 11, 2023 05:15
@codeclimate
Copy link
codeclimate bot commented Jun 11, 2023

Code Climate has analyzed commit b93e37a and detected 18 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 18

View more on Code Climate.

@valerymo
Copy link
Contributor Author

@eguzki , squash and rebase done. Yes, it just 2 files to resolve. Test done - logs attached. Thank you.
finaltestAfterRebase11June.txt

@eguzki eguzki merged commit a0be06e into 3scale:master Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eguzki eguzki eguzki approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@valerymo @eguzki @openshift-merge-robot
0