-
Notifications
You must be signed in to change notification settings - Fork 3
Adding Mapt script tool #2
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Francisco Herrera <[email protected]>
Signed-off-by: Francisco Herrera <[email protected]>
mapt_cluster/README.md
Outdated
| ### Required Tools | ||
| - **Container Engine**: Podman (recommended) or Docker | ||
| - **AWS CLI**: Required when using S3 backing (CI environments) | ||
| - **Pull Secret**: OpenShift pull secret file for cluster creation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've mentioned the "pull secret" in "Required Credentials".
Probably, it could be removed from here, as it's not a tool.
|
|
||
| # Custom cluster configuration | ||
| export CLUSTER_NAME="my-test-cluster" | ||
| export CLUSTER_VERSION="4.18.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since currently, specifying the patch version of the cluster is required, it there a place when available versions could be fetched?
If so, worth mention it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a note in the read me
mapt_cluster/README.md
Outdated
| |--------|-------------| | ||
| | `-c, --create-only` | Create cluster only (don't delete) | | ||
| | `-d, --delete-only` | Delete cluster only (don't create) | | ||
| | `-b, --both` | Create and delete cluster (default) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it worth explain, what is "create and delete" cluster flow.
In my point of view, as someone who looking at it for the first time, I'm not quite understand, what is - create and delete flow.
Shouldn't we first create the cluster, then run the required testing and only then delete the cluster?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the both is to only test the entire workflow in case anything is broken. The idea is to have a complete workflow of creation and deletion
mapt_cluster/README.md
Outdated
|
|
||
| | Option | Description | | ||
| |--------|-------------| | ||
| | `-c, --create-only` | Create cluster only (don't delete) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why to not make the arguments just - "--create" and "--delete"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
mapt_cluster/create_mapt_cluster.sh
Outdated
| container_exists() { | ||
| local container_name="$1" | ||
| local engine="${CONTAINER_ENGINE:-podman}" | ||
| $engine ps -a --format "{{.Names}}" | grep -q "^${container_name}$" 2>/dev/null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it better to surround the var with double quotes here and below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, added
Signed-off-by: Francisco Herrera <[email protected]>
Signed-off-by: Francisco Herrera <[email protected]>
Signed-off-by: Francisco Herrera <[email protected]>
MaxBab
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two tiny comments.
| ``` | ||
|
|
||
| **Finding Available OpenShift Versions**: To find available OpenShift versions with patch numbers, you can: | ||
| - Check using the AWS clie the AMI available for your account with the name openshift-local: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clie -> cli
| AWS_SECRET_ACCESS_KEY AWS secret key for S3 and cluster provisioning | ||
|
|
||
| OPTIONAL ENVIRONMENT VARIABLES: | ||
| AWS_DEFAULT_REGION AWS region for resources (default: us-east-1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a critical, but the indentation in the section below is not aligned. :)
This PR adds a standardized, reusable script for creating and managing OpenShift clusters using MAPT (Managed Application Platform Tools) across different environments: local development to CI/CD pipelines.
Key Features
For more information, please take a look at the Readme file