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.

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

[FLINK-30757] Upgrade busybox version to a pinned version for operator #530

Merged
merged 3 commits into from
Feb 14, 2023

Conversation

xshipeng
Copy link
Contributor

@xshipeng xshipeng commented Feb 10, 2023

What is the purpose of the change

Since e2e test is flaky with latest busybox image in init container, we pinned the image version to a relatively old version. This pull request would like to enable e2e test print init container log and try to upgrade busybox version to the latest pinned version.

Brief change log

  • Add log printing for init container if e2e test is failed.
  • Upgrade the busybox version to the latest in a pinned way.

Verifying this change

This change is already covered by existing tests, such as e2e-tests/test_application_kubernetes_ha.sh.

To reproduce the failed tests locally and test modified init container log printing code:

  • Change the init container command in e2e-tests/data/flinkdep-cr.yaml to command: ['/bin/sh', '-c', 'wget https://repo1.maven.org/maven2/org/apache/flink/flink-examples-streaming_2.12/1.14.4/flink-examples-streaming_2.12-1.14.4.jar -O /flink-artifact/myjob.jar; echo "test error msg"; exit 1;'].
  • Run ./e2e-tests/test_application_kubernetes_ha.sh.
  • Part of the print log will be
...
Flink logs:
Printing init container logs
--------BEGIN CURRENT LOG for flink-example-statemachine-599c95bbf-42wtl:artifacts-fetcher--------
Connecting to repo1.maven.org (198.18.3.208:443)
wget: note: TLS certificate validation not implemented
saving to '/flink-artifact/myjob.jar'
myjob.jar            100% |********************************|  267k  0:00:00 ETA
'/flink-artifact/myjob.jar' saved
test error msg
--------END CURRENT LOG--------
--------BEGIN PREVIOUS LOGS for flink-example-statemachine-599c95bbf-42wtl:artifacts-fetcher--------
Connecting to repo1.maven.org (198.18.3.208:443)
wget: note: TLS certificate validation not implemented
saving to '/flink-artifact/myjob.jar'
myjob.jar            100% |********************************|  267k  0:00:00 ETA
'/flink-artifact/myjob.jar' saved
test error msg
--------END PREVIOUS LOGS--------
Printing main container logs
--------BEGIN CURRENT LOG for flink-example-statemachine-599c95bbf-42wtl:flink-main-container--------
--------END CURRENT LOG--------
Printing init container logs
Printing main container logs
--------BEGIN CURRENT LOG for flink-kubernetes-operator-9744c66bd-27q2r:flink-kubernetes-operator--------
Starting Operator
...

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changes to the CustomResourceDescriptors: no
  • Core observer or reconciler logic that is regularly executed: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable

@xshipeng xshipeng force-pushed the xshipeng/print_init_log branch from 3612645 to dfd1943 Compare February 10, 2023 21:31
@xshipeng
Copy link
Contributor Author

xshipeng commented Feb 10, 2023

Busybox latest (1.36.0) is unstable -> docker-library/busybox#162

@xshipeng xshipeng force-pushed the xshipeng/print_init_log branch from 80233f2 to f8c9d35 Compare February 11, 2023 00:02
@xshipeng xshipeng marked this pull request as ready for review February 11, 2023 00:34
@xshipeng
Copy link
Contributor Author

Hi @gyfora @gaborgsomogyi , the pipeline checks with 1.35.0 busybox version passed. Should we wait for docker-library/busybox#162 to be solved or we are good with 1.35.0 version?

@gaborgsomogyi
Copy link
Contributor

Hi @gyfora @gaborgsomogyi , the pipeline checks with 1.35.0 busybox version passed. Should we wait for docker-library/busybox#162 to be solved or we are good with 1.35.0 version?

I'm fine with 1.35.0 as long as it's stable. The main feature what I was personally missing is the failure log for init containers.

Copy link
Contributor

@gaborgsomogyi gaborgsomogyi left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, decent job. I've tried it out and looks good, only added some comments to make the output easier to read.

e2e-tests/utils.sh Outdated Show resolved Hide resolved
e2e-tests/utils.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@gaborgsomogyi gaborgsomogyi left a comment

Choose a reason for hiding this comment

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

LGTM.

@gaborgsomogyi
Copy link
Contributor

cc @gyfora @mbalassi

@xshipeng
Copy link
Contributor Author

Hi @gyfora, the latest Java integration test failed. Could you please help me retrigger the ci pipeline again? I tested the integration test locally and it passed.

@gyfora gyfora merged commit 02bef3d into apache:main Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants