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

Combine the two controllers into a single pod #717

Merged
merged 11 commits into from
Jun 20, 2023
Merged

Combine the two controllers into a single pod #717

merged 11 commits into from
Jun 20, 2023

Conversation

hectorj2f
Copy link
Collaborator

Summary

Fixes: #631

Release Note

Documentation

Unify our two main controllers into a single pod

@hectorj2f hectorj2f added the enhancement New feature or request label Apr 11, 2023
@hectorj2f hectorj2f self-assigned this Apr 11, 2023
@codecov
Copy link

codecov bot commented Apr 11, 2023 •
edited
Loading

Codecov Report

Merging #717 (8c5af28) into main (b83d530) will decrease coverage by 0.22%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #717      +/-   ##
==========================================
- Coverage   54.98%   54.77%   -0.22%     
==========================================
  Files          45       44       -1     
  Lines        4790     4809      +19     
==========================================
  Hits         2634     2634              
- Misses       1946     1965      +19     
  Partials      210      210
Impacted Files Coverage Δ
cmd/webhook/main.go 0.00% <0.00%> (ø)

@hectorj2f
Copy link
Collaborator Author

Locally works but it seems there is something going on on the github actions.

@hectorj2f hectorj2f force-pushed the unify_controllers branch 2 times, most recently from 80eb843 to 224dc5d Compare April 18, 2023 11:47
@anderssonw
Copy link

anderssonw commented Apr 24, 2023 •
edited
Loading

@hectorj2f Let me know if you need any particular help here. Would love to get this through before we do further work on our use of the controller, as this greatly reduces noice :)

@hectorj2f
Copy link
Collaborator Author

Thanks @anderssonw! I'm still investigating the issue but I have an idea of what is happening.

@hectorj2f
Copy link
Collaborator Author

This PR needs this bug fix to work knative/pkg#2731.

@hectorj2f hectorj2f force-pushed the unify_controllers branch 2 times, most recently from 94c3930 to f8f6c8b Compare June 12, 2023 15:54
@hectorj2f
Copy link
Collaborator Author

We are unblocked from knative issue, I need to check why is failing now.

@hectorj2f
reorganize the code to unify webhooks
86a80f2 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f
remove unnecessary deployment scripts
dc85bef 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f
update ci scripts
5307467 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f
update tests
32c9ffc 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f
update build scripts
b46a2ee 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f
add yaml doc delimeter
4f153fd 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f
set new values for the system resources
7744cad 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f
set the controller options
21f631a 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f
fix: using wrong store api
f67b65f 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f hectorj2f requested review from cpanato and vaikas June 19, 2023 19:12
cpanato
cpanato previously approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

lgtm

just a few nit comments

@cpanato
Copy link
Member

cpanato commented Jun 20, 2023

but the title is in a single container instead of single pod right?

@hectorj2f
Copy link
Collaborator Author

but the title is in a single container instead of single pod right?

@cpanato It is in a single pod. We had two pods before.

@hectorj2f
address reviewer comments
3c6d441 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f
add separator
8c5af28 
Signed-off-by: Hector Fernandez <[email protected]>
@hectorj2f hectorj2f requested a review from cpanato June 20, 2023 09:35
@hectorj2f hectorj2f merged commit 28aff17 into sigstore:main Jun 20, 2023
74 of 76 checks passed
@hectorj2f hectorj2f deleted the unify_controllers branch June 20, 2023 10:20
@github-actions github-actions bot added this to the v1 milestone Jun 20, 2023
ajayk added a commit to wolfi-dev/os that referenced this pull request Jun 21, 2023
@ajayk
Update policy-controller.yaml
714742e 
policy-web hook is merged into policy controller in sigstore/policy-controller#717

Signed-off-by: Ajay Kemparaj <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vaikas vaikas vaikas approved these changes

@cpanato cpanato Awaiting requested review from cpanato

Assignees

@hectorj2f hectorj2f

Labels
enhancement New feature or request
Projects
None yet
Milestone
v1
Development

Successfully merging this pull request may close these issues.

Combine the two controller pods into a single pod
4 participants
@hectorj2f @anderssonw @cpanato @vaikas