[WIP]: Add Cleanup for COS Instances and Improve Script Structure #541
Conversation
ppc64le-cloud-bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aman4433 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ppc64le-cloud-bot
added
the
size/M
| chmod +x /usr/local/bin/pvsadm | ||
| # Function to install pvsadm tool | ||
| install_pvsadm() { | ||
| wget -q -O /usr/local/bin/pvsadm https://github.com/ppc64le-cloud/pvsadm/releases/download/v0.1.1-alpha.7/pvsadm-linux-ppc64le |
There was a problem hiding this comment.
latest version of pvsadm is v0.1.21, please modify/test the script accordingly
| # Function to run IBM Cloud CLI commands | ||
| cleanup_cos() { | ||
| ibmcloud login --apikey "$IBMCLOUD_API_KEY" -r jp-tok || exit 1 |
There was a problem hiding this comment.
this command way expose your apikey into the logs if by chance we enable debugging, please avoid doing,
@Rajalakshmi-Girish lets find some other way..
| for key in $(ibmcloud resource service-keys --instance-name "$instance" | awk 'NR>3 {print $1}'); do | ||
| ibmcloud resource service-key-delete "$key" -f || true | ||
| done | ||
| ibmcloud resource service-instance-delete "$instance" -f || true |
There was a problem hiding this comment.
you are blindly deleting these entries which seems to be risky for me, it will be great if you can delete them where this is initially getting created.
srivastav-abhishek
left a comment
srivastav-abhishek
left a comment
There was a problem hiding this comment.
Thanks for the PR. I have provided few comments, please take a look.
| chmod +x /usr/local/bin/pvsadm | ||
| # Function to install pvsadm tool | ||
| install_pvsadm() { | ||
| wget -q -O /usr/local/bin/pvsadm https://github.com/ppc64le-cloud/pvsadm/releases/download/v0.1.1-alpha.7/pvsadm-linux-ppc64le |
There was a problem hiding this comment.
Going forward, it's better to fetch latest pvsadm from script itself. That way we won't be modifying version here every time new version is released.
| # Function to run IBM Cloud CLI commands | ||
| cleanup_cos() { | ||
| ibmcloud login --apikey "$IBMCLOUD_API_KEY" -r jp-tok || exit 1 |
There was a problem hiding this comment.
There is an ask, not immediate, to move hypershift CI to US datacenters. We will have to change jp-tok to some other region. Please find a way to provide this value as an arg to the script and not hardcoding here.
There was a problem hiding this comment.
or may be read from the file mounted from the secret
4 participants
This PR adds cleanup steps for COS instances and their keys. It also refactors the script to use functions for better readability and maintenance.