fix: eventsource: deterministic volume/volumeMount order #2811
Conversation
|
Let me know if there's something missing, first time doing PR here 🙇 |
popsu
force-pushed
the
eventsource-deterministic-volume-order
branch
from
f98c832 to
40d9ba0
Compare
|
https://github.com/argoproj/argo-events/blob/v1.8.1/common/util.go#L395-L417 I wonder if I should make the change in these functions instead? I don't know the codebase, so not sure at what level should the fix be made. |
The order was non-deterministic earlier and this caused the controller to change the deployment on every reconcile until it the order happened to be same twice in a row. Signed-off-by: Tommi Hovi <[email protected]>
This test is to verify that the order of volume and volumemounts. Without the sort that was added in the previous commit, the order is not deterministic and this test would thus fail some of the time without sorting. Signed-off-by: Tommi Hovi <[email protected]>
popsu
force-pushed
the
eventsource-deterministic-volume-order
branch
from
425ed76 to
a6cefa0
Compare
Signed-off-by: Tommi Hovi <[email protected]>
|
@popsu - thanks for submitting the PR! Could you also add the same logic in the sensor controller? |
Same change that was done to eventsource deployment. Signed-off-by: Tommi Hovi <[email protected]>
…ment Signed-off-by: Tommi Hovi <[email protected]>
Signed-off-by: Tommi Hovi <[email protected]>
popsu
force-pushed
the
eventsource-deterministic-volume-order
branch
from
93744c2 to
786516a
Compare
|
Edit: seems that I don't need to run the
2023/09/25 14:07:48 google/protobuf/descriptor.proto: File not found.
github.com/gogo/protobuf/gogoproto/gogo.proto:32:1: Import "google/protobuf/descriptor.proto" was not found or had errors.
github.com/gogo/protobuf/gogoproto/gogo.proto:38:8: "google.protobuf.EnumOptions" is not defined.
github.com/gogo/protobuf/gogoproto/gogo.proto: "google.protobuf.EnumOptions" is not defined.
[... similar rows sniped...]
github.com/argoproj/argo-events/vendor/k8s.io/apimachinery/pkg/util/intstr/generated.proto:23:1: Import "github.com/gogo/protobuf/gogoproto/gogo.proto" was not found or had errors.
2023/09/25 14:07:48 protoc -I . -I /tmp/tmp.evfHAmKZF6/src -I ./vendor --gogo_out=/tmp/tmp.evfHAmKZF6/src /tmp/tmp.evfHAmKZF6/src/github.com/argoproj/argo-events/vendor/k8s.io/apimachinery/pkg/util/intstr/generated.proto
2023/09/25 14:07:48 Unable to generate protoc on k8s.io.apimachinery.pkg.util.intstr: exit status 1
Makefile:135: recipe for target 'codegen' failed
make: *** [codegen] Error
|
Signed-off-by: Tommi Hovi <[email protected]>
Signed-off-by: Tommi Hovi <[email protected]>
|
✔️ added the same sorting for Sensor deployment. I'm not sure if the Sensor spec has any place (I didn't find with quick look) where the same bug could occur. Since the bug in If there is similar place in Sensor then the testcase I added should be modified (like the one I added for |
Signed-off-by: Tommi Hovi <[email protected]> Signed-off-by: jmillage <[email protected]>
The order was non-deterministic earlier and this caused the controller to change the deployment on every reconcile until it the order happened to be same twice in a row.
Fixes: #2810
I've tested this by making the change to
v1.7.1tag (since we were runningv1.7.1when I made the fix), building the image, and deploying it and the issue went away.Checklist: