Conversation
| "name": "Red Hat Ecosystem Engineering", | ||
| "email": "eco-engineering@redhat.com" | ||
| }, | ||
| "homepage": "https://github.com/dmartinol/ai5-marketplaces", |
There was a problem hiding this comment.
pls update references to the previous GH repo
| @@ -0,0 +1,400 @@ | |||
| --- | |||
| name: error-handling | |||
There was a problem hiding this comment.
maybe a more-specific name to avoid collisions?
| --- | ||
| name: error-handling | ||
| description: | | ||
| Reference guide for common error patterns and recovery strategies in OpenShift S2I deployments. Covers authentication errors (401/403), resource conflicts (409), build failures (git clone, S2I assemble, image push), deployment errors (ImagePullBackOff, CrashLoopBackOff, Pending pods), and route issues. Provides templated responses for each error type with troubleshooting steps and user options. Used by /s2i-build, /deploy, and /containerize-deploy skills. |
There was a problem hiding this comment.
I see that it includes also instructions to handle non 'OpenShift S2I' errors. Maybe we should extend the description to be sure this skill is executed when it's needed?
There was a problem hiding this comment.
I decided to delete the agent - doesn't provide special value that justifies it.
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive "Red Hat Developer" collection that provides end-to-end workflows for containerizing and deploying applications to OpenShift clusters and standalone RHEL systems. The collection includes seven orchestrated skills that guide users through project detection, image selection, building, and deployment with multiple strategies (S2I, Podman, Helm for OpenShift; container/native for RHEL).
Changes:
- Added new rh-developer collection with deployment automation skills for OpenShift and RHEL
- Implemented comprehensive skill documentation covering containerization workflows, build strategies, and deployment orchestration
- Provided template files for Kubernetes resources, Helm charts, and systemd service units
- Included reference documentation for builder images, dynamic validation, session state management, and error handling
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.claude-plugin/marketplace.json |
Registers the rh-developer collection in the marketplace |
rh-developer/.claude-plugin/plugin.json |
Plugin metadata and configuration |
rh-developer/.mcp.json |
MCP server configurations for Kubernetes, Podman, and GitHub |
rh-developer/README.md |
Collection overview |
rh-developer/skills/containerize-deploy/SKILL.md |
Orchestration skill for end-to-end deployment workflow |
rh-developer/skills/s2i-build/SKILL.md |
Source-to-Image build workflow for OpenShift |
rh-developer/skills/deploy/SKILL.md |
Kubernetes resource deployment skill |
rh-developer/skills/helm-deploy/SKILL.md |
Helm-based deployment workflow |
rh-developer/skills/rhel-deploy/SKILL.md |
Standalone RHEL deployment via SSH |
rh-developer/skills/detect-project/SKILL.md |
Project language/framework detection |
rh-developer/skills/recommend-image/SKILL.md |
Intelligent container image recommendation |
rh-developer/templates/systemd/*.service |
Systemd unit templates for container and native deployments |
rh-developer/templates/*.yaml.template |
Kubernetes resource templates |
rh-developer/templates/helm/** |
Helm chart templates |
rh-developer/docs/*.md |
Reference documentation for images, validation, state management, RHEL deployment, Python S2I |
rh-developer/agents/*.md |
Agent documentation for error handling and builder images |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # ${PORT} - Container port mapping (host:container) | ||
| # ${IMAGE} - Container image reference | ||
|
|
||
| [Unit] | ||
| Description=${APP_NAME} Container | ||
| After=network-online.target | ||
| Wants=network-online.target | ||
|
|
||
| [Service] | ||
| Type=simple | ||
| Restart=always | ||
| RestartSec=5 | ||
| ExecStartPre=-/usr/bin/podman stop -t 10 ${APP_NAME} | ||
| ExecStartPre=-/usr/bin/podman rm ${APP_NAME} | ||
| ExecStart=/usr/bin/podman run --name ${APP_NAME} \ | ||
| -p ${PORT}:${PORT} \ |
There was a problem hiding this comment.
Same issue as in systemd-container-rootless.service: The template variable ${PORT} is documented as "Container port mapping (host:container)" but is used as -p ${PORT}:${PORT} which assumes the same port for both host and container. This could cause confusion if users expect to provide a mapping like "8080:3000". Consider either:
- Renaming to ${HOST_PORT} and ${CONTAINER_PORT} with separate variables, or
- Clarifying the documentation to state that ${PORT} represents a single port used for both host and container
| | Helm | `helm_install`, `helm_upgrade`, `helm_status`, `helm_list`, `pod_list` | | ||
| | Rollback | `resources_delete`, `helm_uninstall`, `helm_rollback` | |
There was a problem hiding this comment.
Inconsistent MCP tool naming: In this file, Helm tools are referenced as helm_install, helm_upgrade, helm_status, helm_list, and helm_uninstall. However, in helm-deploy/SKILL.md (lines 358-364), these same tools are referenced with the full prefix as mcp_kubernetes_helm_install, mcp_kubernetes_helm_upgrade, etc. These tool names should be consistent across all skill files. Either use the short form consistently or the full mcp_kubernetes_helm_* prefix consistently.
| | Helm | `helm_install`, `helm_upgrade`, `helm_status`, `helm_list`, `pod_list` | | |
| | Rollback | `resources_delete`, `helm_uninstall`, `helm_rollback` | | |
| | Helm | `mcp_kubernetes_helm_install`, `mcp_kubernetes_helm_upgrade`, `mcp_kubernetes_helm_status`, `mcp_kubernetes_helm_list`, `pod_list` | | |
| | Rollback | `resources_delete`, `mcp_kubernetes_helm_uninstall`, `mcp_kubernetes_helm_rollback` | |
| Type=simple | ||
| User=${SERVICE_USER} | ||
| WorkingDirectory=${APP_PATH} | ||
| Environment=NODE_ENV=production |
There was a problem hiding this comment.
The Environment variable NODE_ENV=production is hardcoded in this template, which is only appropriate for Node.js applications. Since this template is meant to be language-agnostic (as evidenced by the comments listing Node.js, Python, Java, and Go), this environment variable should either be:
- Removed from the template and added conditionally based on detected language, or
- Made into a template variable like ${ENV_VARS} that can be customized per application
This could cause confusion for users deploying non-Node.js applications.
| # ${PORT} - Container port mapping (host:container) | ||
| # ${IMAGE} - Container image reference | ||
|
|
||
| [Unit] | ||
| Description=${APP_NAME} Container | ||
| After=network-online.target | ||
| Wants=network-online.target | ||
|
|
||
| [Service] | ||
| Type=simple | ||
| Restart=always | ||
| RestartSec=5 | ||
| ExecStartPre=-/usr/bin/podman stop -t 10 ${APP_NAME} | ||
| ExecStartPre=-/usr/bin/podman rm ${APP_NAME} | ||
| ExecStart=/usr/bin/podman run --name ${APP_NAME} \ | ||
| -p ${PORT}:${PORT} \ |
There was a problem hiding this comment.
The template variable ${PORT} is documented as "Container port mapping (host:container)" but is used as -p ${PORT}:${PORT} which assumes the same port for both host and container. This could cause confusion if users expect to provide a mapping like "8080:3000". Consider either:
- Renaming to ${HOST_PORT} and ${CONTAINER_PORT} with separate variables, or
- Clarifying the documentation to state that ${PORT} represents a single port used for both host and container
|
|
||
| ```bash | ||
| # Check if an image exists and get metadata | ||
| skopeo inspect docker://registry.access.redhat.com/ubi9/nodejs-20 |
There was a problem hiding this comment.
is there a prerequisites section mentioning the required tools?
is there a system-validation skill implementing the required checks and reporting the validation status?
There was a problem hiding this comment.
No but that's a good idea! I'll add this
There was a problem hiding this comment.
docs should have an header like the following, to allow the parser to detect the original source and put them in the popup details (sorry if I did not mention it before!)
dmartinol
left a comment
dmartinol
left a comment
There was a problem hiding this comment.
That's a really impressive PR 👏 .
I would add a README file to explain the purpose of this collection and the available agents/skills and then I think we're ready to go
|
@ikrispin I merged the PR to validate integration and doc generation. You can fix the issues in a separate PR. Thank you for your first contribution! |
|
@dmartinol Thanks for the review! |
2 participants
No description provided.