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

Add initContainer in Kruize pod to check PostGres pod readiness #1194

Merged
merged 10 commits into from
Jun 14, 2024

Conversation

khansaad
Copy link
Contributor

@khansaad khansaad commented May 14, 2024

Add initContainer in Kruize pod to check postGres pod readiness

Description

This PR will do the following changes -

  • update Kruize deployment section in both minikube and openshift manifest yamls to have an initContainer which checks for the postgres pod service based and starts the main kruize container only when a successful connection is established.

Type of change

  • Refactoring Code
  • New feature

How has this been tested?

  • Openshift ( ResourceHub cluster)/Minikube: Skipped the postgres installation first to deliberately fail the init container and restored it again to see the working of initContainer before starting the kruize pod

Test Configuration

  • Kubernetes clusters tested on: Minikube and Openshift

Checklist 🎯

  • Followed coding guidelines
  • Comments added
  • Dependent changes merged
  • Documentation updated
  • Tests added or updated

Additional information

@khansaad khansaad added the enhancement New feature or request label May 14, 2024
@khansaad khansaad added this to the Kruize 0.0.22_rm Release milestone May 14, 2024
@khansaad khansaad self-assigned this May 14, 2024
@khansaad khansaad force-pushed the kruize-readiness-check branch 2 times, most recently from 758fb40 to 9c3dde1 Compare May 27, 2024 12:20
@khansaad khansaad changed the base branch from master to mvp_demo May 27, 2024 12:21
@khansaad khansaad marked this pull request as ready for review May 27, 2024 12:22
@khansaad
Copy link
Contributor Author

@msvinaykumar Tested it on resource hub cluster, able to see it working on openshift as well

saakhan:autotune$ ./deploy.sh -m crc -c openshift -i quay.io/kruize/autotune_operator:0.0.22_mvp

###   Installing kruize for openshift

use yaml build - 0
Warning: resource namespaces/openshift-tuning is missing the kubectl.kubernetes.io/last-applied-configuration annotation which is required by kubectl apply. kubectl apply should only be used on resources created declaratively by either kubectl create --save-config or kubectl apply. The missing annotation will be patched automatically.
namespace/openshift-tuning configured
serviceaccount/kruize-sa created
clusterrolebinding.rbac.authorization.k8s.io/autotune-scc-crb created
storageclass.storage.k8s.io/manual created
persistentvolume/postgres-pv-volume created
persistentvolumeclaim/postgres-pv-claim created
configmap/kruizeconfig created
Warning: would violate PodSecurity "restricted:latest": allowPrivilegeEscalation != false (container "postgres" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (container "postgres" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or container "postgres" must set securityContext.runAsNonRoot=true), seccompProfile (pod or container "postgres" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
deployment.apps/postgres-deployment created
service/postgres-service created
Warning: would violate PodSecurity "restricted:latest": allowPrivilegeEscalation != false (containers "wait-for-postgres", "kruize" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (containers "wait-for-postgres", "kruize" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or containers "wait-for-postgres", "kruize" must set securityContext.runAsNonRoot=true), seccompProfile (pod or containers "wait-for-postgres", "kruize" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
deployment.apps/kruize created
service/kruize created
cronjob.batch/create-partition-cronjob created
cronjob.batch/kruize-delete-partition-cronjob created
servicemonitor.monitoring.coreos.com/kruize-service-monitor created
configmap/nginx-config created
service/kruize-ui-nginx-service created
Warning: would violate PodSecurity "restricted:latest": allowPrivilegeEscalation != false (container "kruize-ui-nginx-container" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (container "kruize-ui-nginx-container" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or container "kruize-ui-nginx-container" must set securityContext.runAsNonRoot=true), seccompProfile (pod or container "kruize-ui-nginx-container" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
pod/kruize-ui-nginx-pod created
clusterrolebinding.rbac.authorization.k8s.io/kruize-monitoring-view created
Info: Waiting for kruize to come up.....
kruize-578b775f9d-lhbwq               0/1     Init:0/1            0          9s
kruize-578b775f9d-lhbwq               0/1     PodInitializing   0          16s
kruize-578b775f9d-lhbwq               1/1     Running   0          22s
Info: kruize deploy succeeded: Running
kruize-578b775f9d-lhbwq               1/1     Running   0          25s
kruize-ui-nginx-pod                   1/1     Running   0          19s

saakhan:autotune$ oc get pods
NAME                                  READY   STATUS    RESTARTS   AGE
kruize-578b775f9d-lhbwq               1/1     Running   0          32s
kruize-ui-nginx-pod                   1/1     Running   0          26s
postgres-deployment-79cd88468-47p6z   1/1     Running   0          33s

@khansaad khansaad changed the title Add readiness check in Kruize pod for the Postgres pod Add initContainer in Kruize pod to check PostGres pod readiness May 30, 2024
@khansaad khansaad changed the title Add initContainer in Kruize pod to check PostGres pod readiness Add initContainer in Kruize pod to check PostGres pod readiness May 30, 2024
Copy link
Contributor

@msvinaykumar msvinaykumar left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dinogun dinogun left a comment

Choose a reason for hiding this comment

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

LGTM

@dinogun
Copy link
Contributor

dinogun commented Jun 13, 2024

@khansaad can we change the name of the postgres service to kruize-db or something like that. The current naming convention makes this ambiguous as to which deployment that it belong to

Signed-off-by: Saad Khan <saakhan@redhat.com>
Signed-off-by: Saad Khan <saakhan@redhat.com>
Signed-off-by: Saad Khan <saakhan@redhat.com>
Signed-off-by: Saad Khan <saakhan@redhat.com>
@khansaad
Copy link
Contributor Author

@khansaad can we change the name of the postgres service to kruize-db or something like that. The current naming convention makes this ambiguous as to which deployment that it belong to

Updated now!

@dinogun
Copy link
Contributor

dinogun commented Jun 14, 2024

@khansaad can we change the name of the postgres service to kruize-db or something like that. The current naming convention makes this ambiguous as to which deployment that it belong to

Updated now!

@khansaad Thanks for the update, can you also change the name of postgres-deployment

Signed-off-by: Saad Khan <saakhan@redhat.com>
@khansaad
Copy link
Contributor Author

@khansaad can we change the name of the postgres service to kruize-db or something like that. The current naming convention makes this ambiguous as to which deployment that it belong to

Updated now!

@khansaad Thanks for the update, can you also change the name of postgres-deployment

Done!

@khansaad khansaad requested a review from dinogun June 14, 2024 08:35
@dinogun
Copy link
Contributor

dinogun commented Jun 14, 2024

@khansaad Sorry can you grep for the name postgres in the manifest files and update it something more relevant wherever appropriate, thanks!

Signed-off-by: Saad Khan <saakhan@redhat.com>
Copy link
Contributor

@dinogun dinogun left a comment

Choose a reason for hiding this comment

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

LGTM

@dinogun
Copy link
Contributor

dinogun commented Jun 14, 2024

@khansaad Looks like the PR check test scripts might need to be updated as well

Signed-off-by: Saad Khan <saakhan@redhat.com>
Signed-off-by: Saad Khan <saakhan@redhat.com>
Signed-off-by: Saad Khan <saakhan@redhat.com>
Copy link
Contributor

@chandrams chandrams left a comment

Choose a reason for hiding this comment

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

LGTM

@khansaad khansaad requested a review from dinogun June 14, 2024 12:24
@dinogun dinogun merged commit cac92d7 into kruize:mvp_demo Jun 14, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants