-
Notifications
You must be signed in to change notification settings - Fork 95
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
Threescale 7668 #778
Threescale 7668 #778
Conversation
c923328
to
3d1c28c
Compare
@austincunningham would you mind addressing the remaining comments? Not sure I can do much without a cluster. FYI some tests are failing cause we are using porta-client v0.6 which doesnt have the new changes, think we might need a new tag for porta @eguzki |
03c243f
to
c86d304
Compare
/retest |
c10ed50
to
c962b03
Compare
/ok-to-test |
32a0821
to
0c9bf0d
Compare
verification steps working 💪 |
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.
Looking good!
I left some comments.
Awesome work testing the app controller logic.
I see there is some doc for the Application CRD reference. It would be very nice to have some higher level as user guide. Also useful for downstream doc. In https://github.com/3scale/3scale-operator/blob/master/doc/operator-application-capabilities.md,
- add Application CRD to the index, with link to the ref doc.
- add few examples
- Some "howto" user guide, very similar to the verification steps, so the end user can easily follow steps to deploy an application.
I realized that we missed this for the ProxyConfigPromote CRD
0c9bf0d
to
3bfd6d9
Compare
heads up, after merging #785, this PR would need to be rebased and some (hopefully few and small) changes should be applied. |
Create, Read and Update basics working Fixing nil pointer and adding suspend/resume functionality Adding external resources check and changing application status Addressing comments Add documentation remove json unmarshal and check for invalid status move the account and product get to applicationReconciler remove generated test-suite file update to 3scale-porta-go-client v0.7.0 make bundle add basic unit tests for create and delete application CR test for suspension/live and change plan ID update the status on the developer account not found
3bfd6d9
to
fff32b3
Compare
Code Climate has analyzed commit 715c23d and detected 40 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
/test test-e2e |
1 similar comment
/test test-e2e |
Awesome work @austincunningham 🎖️ |
I spotted small nitpicks, but we can work with them in coming PR's |
What
THREESCALE-7668
Addition of new application CR, that supports :
Verification
As this PR with new functionality hasnt been merged yet, check out this branch of porta client locally.Check out this 3scale-operator branch locally and add the following line of code to go.mod. (This will use the local version of the porta client setup in step1)9.Create an Invalid Application CR , status should give out about invalid account and product CR names.