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.

Sign up for GitHub

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

Jump to bottom

Add zync controller #5

Merged
merged 38 commits into from
Jun 24, 2020
Merged

Add zync controller #5

merged 38 commits into from
Jun 24, 2020

Conversation

raelga
Copy link
Contributor

@raelga raelga commented Jun 19, 2020 •
edited
Loading

Adds the zync controller.

  • Reference documentation
  • CRD spec
  • Role templates
    • Deployments
    • Config maps
    • Secrets
    • Services
    • Monitoring dashboards
  • Role Defaults
  • Updated common documentation
  • Test and validation

@raelga raelga requested review from roivaz and slopezz June 19, 2020 06:00
@raelga raelga self-assigned this Jun 19, 2020
@raelga raelga force-pushed the feature/add-zync branch from 4faf11b to a02d046 Compare June 19, 2020 11:54
@raelga raelga force-pushed the feature/add-zync branch from c5951a2 to 853ba8a Compare June 22, 2020 09:39
slopezz
slopezz reviewed Jun 22, 2020
View reviewed changes
docs/zync-crd-reference.md Outdated Show resolved Hide resolved
slopezz
slopezz reviewed Jun 22, 2020
View reviewed changes
docs/zync-crd-reference.md Outdated Show resolved Hide resolved
slopezz
slopezz reviewed Jun 22, 2020
View reviewed changes
docs/zync-crd-reference.md Outdated Show resolved Hide resolved
slopezz
slopezz reviewed Jun 22, 2020
View reviewed changes
docs/zync-crd-reference.md Outdated Show resolved Hide resolved
raelga and others added 7 commits June 22, 2020 19:18
@raelga
Remove duplicated line
5c7ba0f
@raelga @slopezz
Add missing default value
67d7f3c 
Co-authored-by: slopezz <41513123+slopezz@users.noreply.github.com>
@raelga @slopezz
Add missing default value
4d18cec 
Co-authored-by: slopezz <41513123+slopezz@users.noreply.github.com>
@raelga
Move version section to the end
905756d
@raelga
Remove echo task
33f141a
@raelga
Rename image version to image tag
c0ecb7b
@raelga
Quote resources
8d96e0b
@raelga raelga force-pushed the feature/add-zync branch from 28ab270 to 14f3aab Compare June 23, 2020 08:01
@raelga raelga force-pushed the feature/add-zync branch from 14f3aab to 8280b17 Compare June 23, 2020 08:26
roivaz
roivaz approved these changes Jun 23, 2020
View reviewed changes
Copy link
Member

@roivaz roivaz left a comment

Choose a reason for hiding this comment

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

Good work!

One thing that I have seen is that https://github.com/3scale/saas-operator/blob/8280b1765a8e65e3dbd14c48b6b17230b683b091/deploy/role.yaml needs some cleanup. There are several permissions that are not used at all and also several repeated ones. The operator-sdk does not really do a very good job updating this file and I usually just end up managing it myself to keep the permissions down to those that are strictly required.

This is probably something that can be tackled in a different issue/PR.

roles/zync/defaults/main.yml Show resolved Hide resolved
slopezz
slopezz approved these changes Jun 23, 2020
View reviewed changes
Copy link
Member

@slopezz slopezz left a comment

Choose a reason for hiding this comment

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

Thank you very much for applying my suggestions, really nice work!! :D

/lgtm

@raelga raelga merged commit 51e4eea into master Jun 24, 2020
@raelga raelga deleted the feature/add-zync branch June 24, 2020 07:09
@raelga raelga mentioned this pull request Jun 25, 2020
Closed
2 tasks
@raelga raelga added kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple sprints to complete. size/M Requires about a day to complete the PR or the issue. labels Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slopezz slopezz slopezz approved these changes

@roivaz roivaz roivaz approved these changes

Assignees

@raelga raelga

Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple sprints to complete. size/M Requires about a day to complete the PR or the issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@raelga @roivaz @slopezz