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

Fixed Otel Controller - Auxiliares resources deletion #2575

Merged
merged 31 commits into from Feb 28, 2024
Merged

Fixed Otel Controller - Auxiliares resources deletion #2575

merged 31 commits into from Feb 28, 2024

Conversation

yuriolisa
Copy link
Contributor

@yuriolisa yuriolisa commented Jan 29, 2024 •
edited

Description:

Fixed HPA deletion
Link to tracking Issue:
Resolves #2568
Resolves #2651
Resolves #2587

Testing:
Added new e2e test to check this step.
Documentation:

  • Created a new bundle due to the new RBAC for PersistentVolumes and PersistentVolumesClaims.
  • Fixed the labels used by Ingress resources and their unit tests.
  • Created new function findOtelOwnedObjects which will find all the owned objects and serve as an auxiliary resource to the reconcileDesiredObjects function to keep or delete the expected objects.
  • Changed the parameters of the function HorizontalPodAutoscaler from client.Object to *autoscalingv2.HorizontalPodAutoscaler to keep the same pattern we have been using for another resources.
  • Added the networkv1 package to the scheme. (networkingv1.AddToScheme(scheme)).
  • Update Chainsaw from 0.1.4 to 0.1.7 in order to get the patch operation enabled.
  • Created new e2e tests for Ingress and HorizontalPodAutoscalers to test the deletion process.
  • Added the testdata.ServiceMonitorCRD, testdata.PodMonitorCRD to the controllers/suite_test.go

@yuriolisa
Fixed HPA deletion
5a31481 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa yuriolisa requested a review from a team as a code owner January 29, 2024 15:28
@yuriolisa
Fix e2e tests
bd9bd93 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Added reconciliation test
10f82b0 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Changed from client.Object to HPA Obj
515a034 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Added owned objects function
2ac2af7 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixed controller test
a61de02 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixed controller test
20c7bf6 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixed controller test
29135cd 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
jaronoff97
jaronoff97 reviewed Feb 13, 2024
View reviewed changes
Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

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

Looking great! Thank you for implementing this.

controllers/opentelemetrycollector_controller.go Show resolved Hide resolved
tests/e2e-autoscale/autoscale/04-error.yaml Show resolved Hide resolved
controllers/common.go Outdated Show resolved Hide resolved
controllers/common.go Outdated Show resolved Hide resolved
@yuriolisa
Add PodDisruptionBudget
8f6159c 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
jaronoff97
jaronoff97 reviewed Feb 14, 2024
View reviewed changes
Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

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

one comment. Otherwise, were you able to confirm locally that this does as we intend?

controllers/common.go Outdated Show resolved Hide resolved
@yuriolisa
Add PodDisruptionBudget
dacc99a 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
pavolloffay
pavolloffay reviewed Feb 16, 2024
View reviewed changes
controllers/common.go Outdated Show resolved Hide resolved
controllers/common.go Outdated Show resolved Hide resolved
@yuriolisa
Change vars setting
a0a30da 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Change vars setting
1edd721 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Change vars setting
29ad837 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@jaronoff97 jaronoff97 mentioned this pull request Feb 20, 2024
Closed
@yuriolisa
Update e2e tests to chainsaw
2f1b576 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa yuriolisa changed the title Fixed HPA deletion Fixed Otel Controller - Auxiliares resources deletion Feb 20, 2024
@yuriolisa
Added ingress e2e tests
9a669d0 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
jaronoff97
jaronoff97 reviewed Feb 21, 2024
View reviewed changes
Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

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

just two small things, otherwise this LGTM!! Thank you so much Yuri.

controllers/common.go Outdated Show resolved Hide resolved
controllers/common.go Outdated Show resolved Hide resolved
@yuriolisa
Added Volumes, changed function's place
ebbb447 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@pavolloffay
Copy link
Member

The CI is still failing :/

jaronoff97
jaronoff97 approved these changes Feb 26, 2024
View reviewed changes
Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

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

looks like it's failing on: error listing ServiceMonitors: no kind is registered for the type v1.ServiceMonitorList in scheme "pkg/runtime/scheme.go:100"

Overall though i think this looks good to me!

yuriolisa and others added 11 commits February 26, 2024 19:37
@yuriolisa
Added Volumes, changed function's place
55240ba 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Added Volumes, changed function's place
4350a56 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixed Suite Test
1839504 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Added RBAC for Volumes
88b30ef 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Added RBAC for Volumes
7e1b010 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixed Suite Test
18c8385 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixed Suite Test
3e77e72 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixed Suite Test
8bd21d5 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@jaronoff97
Add a sleep
2087366
@yuriolisa
Merge pull request #1 from jaronoff97/fix-hpa-delete
d2a0269 
Add a sleep to the tests
@yuriolisa
Fixed Chainsaw test
7d09dd0 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@pavolloffay
Copy link
Member

Still one e2e test is failing

@yuriolisa
Fixed Chainsaw test
42469b4 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
swiatekm-sumo
swiatekm-sumo reviewed Feb 28, 2024
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@yuriolisa
Fixed Chainsaw test
8af8038 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixed Chainsaw test
1ffc7eb 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixed Chainsaw test
ba7c3b2 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@jaronoff97
Copy link
Contributor

@yuriolisa thank you so much for your heroics here. I really appreciate the work and all the fixes that come from this.

@jaronoff97 jaronoff97 merged commit 874c72a into open-telemetry:main Feb 28, 2024
29 checks passed
@swiatekm-sumo swiatekm-sumo mentioned this pull request Mar 4, 2024
Merged
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
@yuriolisa @jaronoff97
Fixed Otel Controller - Auxiliares resources deletion (open-telemetry…
0ef71e3 
…#2575)

* Fixed HPA deletion

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fix e2e tests

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Added reconciliation test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Added owned objects function

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed controller test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed controller test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed controller test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Add PodDisruptionBudget

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Change vars setting

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Change vars setting

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Update e2e tests to chainsaw

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Added ingress e2e tests

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Added Volumes, changed function's place

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Added Volumes, changed function's place

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Added Volumes, changed function's place

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed Suite Test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Added RBAC for Volumes

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Added RBAC for Volumes

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed Suite Test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed Suite Test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed Suite Test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Add a sleep

* Fixed Chainsaw test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed Chainsaw test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed Chainsaw test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixed Chainsaw test

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

---------

Signed-off-by: Yuri Sa <yurimsa@gmail.com>
Co-authored-by: Jacob Aronoff <jacobaronoff45@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swiatekm-sumo swiatekm-sumo swiatekm-sumo approved these changes

@pavolloffay pavolloffay pavolloffay approved these changes

@jaronoff97 jaronoff97 jaronoff97 approved these changes

@frzifus frzifus Awaiting requested review from frzifus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@yuriolisa @pavolloffay @jaronoff97 @frzifus @swiatekm-sumo