-
Notifications
You must be signed in to change notification settings - Fork 309
fix: fix the e2e test script for fluentd #1791
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
Conversation
e768108 to
d647fa0
Compare
a26fb8e to
87b92da
Compare
| @@ -1 +1 @@ | |||
| v3.5.0 | |||
| latest No newline at end of file | |||
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.
Don't think this change belongs to this PR
| latest | |
| v3.5.0 | |
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.
it will block the test to run successfully if we don't fix it. see here
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.
Changing this will break stuff like releases, we rather fix it in the script by doing a sed replacement or something like that.
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.
No, the release #1741 should be actually be merged into a separate release branch and keep the master's version to latest. There was a mistake that we didn't create a release branch for v3.5.0
Please check the file history and you will know.
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.
Ok. Guess that is fine for now, IMHO we should change that release process.
Just make releases straight on main. if there is ever a need to hotfix a release, we can always create the branch from the last minor tag. hope to get some time redesigning the release automation and the manual process to simplify this. It is overly complicated imho.
apis/fluentd/v1alpha1/tests/expected/fluentd-namespaced-cfg-filter-selector.cfg
Outdated
Show resolved
Hide resolved
apis/fluentd/v1alpha1/tests/expected/fluentd-namespaced-cfg-filter-output-selector.cfg
Outdated
Show resolved
Hide resolved
bc9fdb8 to
4c46ec8
Compare
marcofranssen
left a comment
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.
Couple suggestions to simplify
6e95e26 to
ab8acf5
Compare
0c99b62 to
6f2fc45
Compare
|
Hi @marcofranssen , please fix the DCO issue for the last commit and also the e2e-test itself, right now it still fails due to the changes in last commit. |
Signed-off-by: Chengwei Guo <[email protected]>
Signed-off-by: Chengwei Guo <[email protected]>
Signed-off-by: Chengwei Guo <[email protected]>
Signed-off-by: Chengwei Guo <[email protected]>
Signed-off-by: chengweiguo <[email protected]>
Signed-off-by: chengweiguo <[email protected]>
Signed-off-by: chengweiguo <[email protected]>
Signed-off-by: Marco Franssen <[email protected]>
Signed-off-by: chengweiguo <[email protected]>
b3de495 to
e03751e
Compare
Sorry will have a look asap. Guess mistake in my review suggestion. |
Signed-off-by: Marco Franssen <[email protected]>
Signed-off-by: Marco Franssen <[email protected]>
Signed-off-by: Marco Franssen <[email protected]>
Signed-off-by: Marco Franssen <[email protected]>
Signed-off-by: Marco Franssen <[email protected]>
Signed-off-by: Marco Franssen <[email protected]>
Signed-off-by: Marco Franssen <[email protected]>
|
I have fixed the failures, but the script which I didn't touch is still having failures that are absorbed. https://github.com/fluent/fluent-operator/actions/runs/19875104106/job/56960475894?pr=1791#step:7:192 |
Signed-off-by: Marco Franssen <[email protected]>
|
Did an attempt to fix it but seems the recent refactor has some gaps. @joshuabaird could you pitch in and have a look on fixing that part? |
marcofranssen
left a comment
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.
LGTM 🚀
- Fixed the failures regarding make tasks.
- Filed an issue for the helm chart dependency problem #1812
Still need to do some more cleanup on the scripts and make tasks to make it bit more structured and predictable for contributors. Setup is currently more complicated then it should be, and make tasks are not all correctly depending on each other, causing difficulty to know what to run in what order.
What this PR does / why we need it:
As titled, after this fix, we can run
make e2esuccessfully.