Skip to content
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 test bash scripts #668

Merged
merged 10 commits into from Jan 22, 2018
Merged

Update test bash scripts #668

merged 10 commits into from Jan 22, 2018

Conversation

jmrodri
Copy link
Contributor

@jmrodri jmrodri commented Jan 19, 2018

During async bind, I had a set of bash scripts: bind.sh, unbind.sh, getbind.sh, and getinstance.sh which worked with the broker's route. They were duplicates of the original bash scripts in the test folder.

The older scripts were also outdated, so I brought them up to date with my newer scripts. They now also take in parameters for some of the arguments. They are not perfect but get the job done.

This is a start to update the test scripts that could be useful for
testing out the API directly.
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 19, 2018
@jmrodri jmrodri added build tech-debt 3.9 | release-1.1 Kubernetes 1.9 | Openshift 3.9 | Broker release-1.1 labels Jan 19, 2018
Copy link
Contributor

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

Is this going to take the place of asbcli?

test/bind.sh Outdated
. shared.sh
INSTANCE_ID=$1
BINDING_ID=$2
PLAN_UUID="7f4a5e35e4af2beb70076e72fab0b7ff"
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it is hardcoded I assume there is a registry config that we must use to get this to work. I think it might be worth documenting in the README.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shawn-hurley heh it was hardcoded because on a few of the calls we don't really care apparently. Clearly this is NOT going to work with provision. I'll look into it.

http://localhost:1338/v2/service_instances/$instanceUUID/service_bindings/$bindingUUID
-k \
-X PUT \
-H "Authorization: bearer $(oc whoami -t)" \
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, make sure the user knows they must be logged into the cluster and have the right permissions.

Copy link
Member

Choose a reason for hiding this comment

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

And you can't be a system:admin for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added to the readme

-H "Authorization: bearer $(oc whoami -t)" \
-H "Content-type: application/json" \
-H "Accept: application/json" \
-H "X-Broker-API-Originating-Identity: " \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're going to need to set auto_escalate=true for these scripts to work, I would also document that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added to the readme

Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

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

I see a couple of comments from @shawn-hurley that are worth addressing. Other than that, this LGTM.

@jmrodri
Copy link
Contributor Author

jmrodri commented Jan 19, 2018

@shawn-hurley was not supposed to replace asbcli. I guess it could since I haven't used asbcli in ages. Does it even work anymore? Just some generally useful scripts to do curl against the broker's api.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 02e3d5a on updatescripts into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 99ee057 on updatescripts into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 99ee057 on updatescripts into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 205563f on updatescripts into ** on master**.

@coveralls
Copy link

coveralls commented Jan 20, 2018

Coverage Status

Changes Unknown when pulling 205563f on updatescripts into ** on master**.

@jmrodri jmrodri merged commit d5e0fa2 into master Jan 22, 2018
@jmrodri jmrodri deleted the updatescripts branch January 22, 2018 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 | release-1.1 Kubernetes 1.9 | Openshift 3.9 | Broker release-1.1 build size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tech-debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants