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

[RHCLOUD-27862] add Pod Lifecyle hooks #830

Merged
merged 7 commits into from
Nov 3, 2023

Conversation

maskarb
Copy link
Member

@maskarb maskarb commented Jun 14, 2023

https://issues.redhat.com/browse/RHCLOUD-27862

minimal example (use the example clowdenv.yml):

apiVersion: cloud.redhat.com/v1alpha1
kind: ClowdApp
metadata:
  name: hello
  namespace: jumpstart
spec:
  envName: env-jumpstart
  deployments:
  - name: app
    podSpec:
      image: nginx
      terminationGracePeriodSeconds: 500
      lifecycle:
        postStart:
          exec:
            command:
            - "/bin/sh"
            - "-c"
            - echo "$(date -Iseconds) ...MESSAGE FROM THE POSTSTART HOOK..." > /proc/1/fd/1
        preStop:
          exec:
            command:
              - "/bin/sh"
              - "-c"
              - echo "$(date -Iseconds) ...MESSAGE FROM THE PRESTOP HOOK..." > /proc/1/fd/1
    webServices:
      private:
        enabled: true
      public:
        enabled: true

(These hooks are a variation from: https://thecloudblog.net/lab/kubernetes-container-lifecycle-events-and-hooks/. The echo "message" > /proc/1/fd/1 prints that message to the pod logs so we can see the hooks in action)

The deployment generated by this clowdapp contains:

...
          lifecycle:
            postStart:
              exec:
                command:
                  - /bin/sh
                  - '-c'
                  - echo "$(date -Iseconds) ...MESSAGE FROM THE POSTSTART HOOK..." > /proc/1/fd/1
            preStop:
              exec:
                command:
                  - /bin/sh
                  - '-c'
                  - echo "$(date -Iseconds) ...MESSAGE FROM THE PRESTOP HOOK..." > /proc/1/fd/1
          ...
      terminationGracePeriodSeconds: 500
...

Using this image and lifecycle hooks, open the pod terminal and see the printed log messages. The readiness/liveness probes will fail for this container. So when the pod restarts, we can see the preStop hook just before the sigquit was received:

2023/11/01 21:44:08 [notice] 1#1: start worker process 34
2023-11-01T21:44:08+00:00 ...MESSAGE FROM THE POSTSTART HOOK...
2023-11-01T21:45:37+00:00 ...MESSAGE FROM THE PRESTOP HOOK...
2023/11/01 21:45:37 [notice] 1#1: signal 3 (SIGQUIT) received, shutting down

@psav
Copy link
Collaborator

psav commented Jun 16, 2023

Can you run make pre-push on this please?

@psav psav added enhancement New feature or request pr-architectural-change Required architect sign-off labels Jun 16, 2023
@maskarb
Copy link
Member Author

maskarb commented Jun 16, 2023

Can you run make pre-push on this please?

Done!

I will note, I have not done any testing of these changes at all yet. I was going to see if I can get this working, and then see if yall had any thoughts on these changes to the Spec.

@maskarb maskarb force-pushed the pod-lifecycle-hooks branch 2 times, most recently from 8e72059 to 1ebda3f Compare October 17, 2023 00:17
@psav
Copy link
Collaborator

psav commented Oct 20, 2023

I have no problem with the spec changes

@maskarb maskarb changed the title add Pod Lifecyle hooks [RHCLOUD-27862] add Pod Lifecyle hooks Nov 1, 2023
@maskarb maskarb marked this pull request as ready for review November 1, 2023 16:22
Copy link
Contributor

@adamrdrew adamrdrew left a comment

Choose a reason for hiding this comment

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

LGTM really, but a couple of questions before approval

tests/kuttl/test-lifecycle-hook/01-pods.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@bsquizz bsquizz left a comment

Choose a reason for hiding this comment

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

Sorry just a couple minor doc generator things again

bsquizz
bsquizz previously approved these changes Nov 2, 2023
Copy link
Contributor

@bsquizz bsquizz left a comment

Choose a reason for hiding this comment

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

I'll sort out those doc generation shenanigans later, it doesn't need to hold up this PR

@psav
Copy link
Collaborator

psav commented Nov 3, 2023

Pending test outcome I'm happy to merge this

@psav
Copy link
Collaborator

psav commented Nov 3, 2023

/retest

@adamrdrew adamrdrew merged commit 702169f into RedHatInsights:master Nov 3, 2023
7 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 pr-architectural-change Required architect sign-off
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants