Skip to content

openshift: deployment improvements (keytab migration, Recreate strategy, dist-git SSH fix) - #487

Merged
TomasTomecek merged 5 commits into
packit:mainfrom
TomasTomecek:depl-6
May 18, 2026
Merged

TomasTomecek merged 5 commits into
packit:mainfrom
TomasTomecek:depl-6

Conversation

@TomasTomecek

Copy link
Copy Markdown
Member

Summary

  • Migrate keytab from jotnar-bot to redhat-ymir-agent in mcp-gateway deployment and kerberos ConfigMap
  • Switch all deployments from RollingUpdate to Recreate strategy — with replicas=1 and a tight namespace quota, RollingUpdate causes a deadlock where old failing pods hold quota and prevent new pods from being created
  • Add User redhat-ymir-agent to the dist-git SSH config for pkgs.devel.redhat.com (was only set for the bastion)
  • Show running pod images and ImageStream import times at end of deploy.sh
  • Document how to manually trigger image rebuilds via the GitLab CI jobs view

Test plan

  • ./openshift/deploy.sh runs cleanly
  • mcp-gateway pod comes up and initializes Kerberos ticket for redhat-ymir-agent
  • dist-git SSH access works from the mcp-gateway pod

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request renames the bot keytab to redhat-ymir-agent across various OpenShift configurations, updates the internal SSH configuration, and switches the deployment strategy for several services from RollingUpdate to Recreate. It also adds a helper function to the deployment script to display running pod images and ImageStream tags. Feedback identifies that the new status check in the deployment script may show stale information due to the asynchronous nature of OpenShift deployments and suggests improving the pod output to display human-readable image tags and support multiple containers.

Comment thread openshift/deploy.sh Outdated
Comment on lines +39 to +41
oc get pods \
-o custom-columns='POD:.metadata.name,IMAGE:.status.containerStatuses[0].imageID' \
--sort-by='.metadata.name'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using .status.containerStatuses[0].imageID shows the resolved image SHA, which is difficult to verify at a glance (e.g., docker-pullable://...). Additionally, it only shows the first container, which might be incomplete if sidecars are added later. Using .spec.containers[*].image provides the human-readable tags and supports multiple containers. Including the pod status is also helpful to see if pods are still restarting.

Suggested change
oc get pods \
-o custom-columns='POD:.metadata.name,IMAGE:.status.containerStatuses[0].imageID' \
--sort-by='.metadata.name'
oc get pods \
-o custom-columns='POD:.metadata.name,IMAGE:.spec.containers[*].image,STATUS:.status.phase' \
--sort-by='.metadata.name'

Comment thread openshift/deploy.sh Outdated
# apply configmap-jira-issue-fetcher-env.yml
# apply cronjob-jira-issue-fetcher.yml

show_running_images

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since oc apply is asynchronous, calling show_running_images immediately after will likely show the state of the pods before the new deployment has completed. With the Recreate strategy, pods are deleted before new ones are created, so this output might show pods in Terminating state or no pods at all for a brief moment. Consider adding a brief sleep or using oc rollout status for critical deployments to ensure the summary reflects the new state.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I agree with Gemini, the idea was great but the output is not useful, I'm gonna drop that commit and send a better version in the next PR

majamassarini
majamassarini previously approved these changes May 15, 2026

@majamassarini majamassarini left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🙏🏻

TomasTomecek and others added 5 commits May 18, 2026 10:31
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>
Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>
All our deployments run with replicas=1, making RollingUpdate
counterproductive: it holds the old pod alive until the new one is
ready, which exhausts the namespace memory quota and causes the new pod
to fail with FailedCreate. This creates a deadlock that requires manual
intervention on every deploy.

Recreate simply terminates the old pod first and then starts the new
one, which is the right behavior for single-replica workloads.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add explicit requests.memory to all agent deployments (previously
unset, defaulting to limits). Bump backport agents from 4Gi to 5Gi
limits using the newly available headroom.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@TomasTomecek
TomasTomecek merged commit 369a373 into packit:main May 18, 2026
9 checks passed
@TomasTomecek
TomasTomecek deleted the depl-6 branch May 18, 2026 15:15
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 this pull request may close these issues.

2 participants