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
Update scripts to use the broker apb #891
Conversation
Testing for Kubernetes |
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.
Requesting changes because I actually just have questions but thats not an option :)
Overall this is a big 👍 from me!
|
|
||
| create-broker-resource | ||
| kubectl create ns $BROKER_NAMESPACE | ||
| kubectl create serviceaccount $APB_NAME --namespace $BROKER_NAMESPACE |
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.
Can we delete this svcacct after this runs?
Also could this just run as the user running the script? do we want to do this extra work?
| -p NAMESPACE=ansible-service-broker \ | ||
| $VARS -f - | oc create -f - | ||
| # Use the automation-broker-apb to deploy the broker | ||
| oc create serviceaccount $APB_NAME --namespace $BROKER_NAMESPACE |
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.
Same question as below re: do we need the service account
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 actually was smart and delete that stuff in this script but forgot to add that to the k8s version.
scripts/run_latest_build.sh
Outdated
| exit | ||
| fi | ||
| oc delete pod -n $BROKER_NAMESPACE $APB_NAME | ||
| oc delete clusterrolebinding $APB_NAME | ||
| oc delete serviceaccount $APB_NAME |
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 your going to want to specify the namespace for this deletion as well.
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.
ACK, the regex is probably okay don't bother trying to fix it.
|
|
||
| version=$(oc version | head -1) | ||
| client_version=$(echo $version | egrep -o 'v[0-9]+(\.[0-9]+)+' | tr -d v.) |
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 assume that the third number in the version can never be more than 9? i.e. v3.9.10-alpha.0+221863f-734 because that will fail the check we have for $client_version. It will consider 3.9.10 to be greater than 3.10.0 which is wrong.
$ version="oc v3.9.10-alpha.0+221863f-734"
$ echo $version | egrep -o 'v[0-9]+(\.[0-9]+)+' | tr -d v.
3910
Not a blocker but something to remember if it breaks in the future.
Update scripts that deploy the broker to use the
automation-broker-apb.