Fix intermittent build failures #8312

vishesh92 · 2023-12-06T09:23:32Z

Description

This PR fixes intermittent build failures which happen due to parallel test runs. As of now port 9399 is being used in NetworkerClientTest & VeeamClientTest. This PR updates the port in NetworkerClientTest from 9399 to 9400.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

rohityadavcloud · 2023-12-06T09:25:04Z

Could we somehow use random/available ports, there has to be a way to do that @vishesh92 ?

DaanHoogland

lgtm

DaanHoogland · 2023-12-06T09:26:28Z

I don t think we should assign random ports, but make it deterministic. random is a sure way to get intermitted results in the end.

codecov · 2023-12-06T09:31:31Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a15b706) 22.25% compared to head (1b4c9be) 27.92%.
Report is 9 commits behind head on main.

❗ Current head 1b4c9be differs from pull request most recent head 1449aa3. Consider uploading reports for the commit 1449aa3 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8312      +/-   ##
============================================
+ Coverage     22.25%   27.92%   +5.67%     
- Complexity    22439    29729    +7290     
============================================
  Files          5117     5251     +134     
  Lines        346819   368797   +21978     
  Branches      49790    53765    +3975     
============================================
+ Hits          77190   102993   +25803     
+ Misses       258466   251569    -6897     
- Partials      11163    14235    +3072

Flag	Coverage Δ
simulator-marvin-tests	`23.48% <ø> (-0.36%)`	⬇️
uitests	`4.44% <ø> (ø)`
unit-tests	`14.89% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

vishesh92 · 2023-12-06T09:37:11Z

Could we somehow use random/available ports, there has to be a way to do that @vishesh92 ?

I tried using wiremock's dynamicPort, but for some reason it's not working.

kishankavala · 2023-12-06T09:45:14Z

Can you create a common util similar to the following to find a free port:

try (ServerSocket socket = new ServerSocket(0)) {
            socket.setReuseAddress(true);
            return socket.getLocalPort();
        }

DaanHoogland · 2023-12-06T09:45:45Z

Could we somehow use random/available ports, there has to be a way to do that @vishesh92 ?

I tried using wiremock's dynamicPort, but for some reason it's not working.

If dynamic ports are using a global registry it might work but predictability always suffers. Let's stick to this.

weizhouapache · 2023-12-06T10:06:12Z

Can you create a common util similar to the following to find a free port:
try (ServerSocket socket = new ServerSocket(0)) {
            socket.setReuseAddress(true);
            return socket.getLocalPort();
        }

+1

btw, can we specify a port range ?

shwstppr

LGTM

shwstppr · 2023-12-06T11:01:03Z

@vishesh92 will you be adding suggested changes in this PR or can it be done as a separate PR?

vishesh92 · 2023-12-06T11:20:29Z

@shwstppr Let me make the change in this PR itself.

vishesh92 · 2023-12-06T11:29:15Z

@kishankavala @shwstppr @DaanHoogland I have made the changes to randomly select a port.

kishankavala · 2023-12-06T12:06:14Z

plugins/backup/networker/src/test/java/org/apache/backup/networker/NetworkerClientTest.java

@@ -60,6 +63,15 @@ public void setUp() throws Exception {
        client = new NetworkerClient(url, adminUsername, adminPassword, false, 60);
    }

+    private int getRandomPort() {


This can be in a util class, so that other tests can also use it.

LGTM otherwise

Agree. But let's do that in a separate PR.

I think we should not do this. let's keep it deterministic.

kishankavala · 2023-12-06T12:09:44Z

Can you create a common util similar to the following to find a free port:
try (ServerSocket socket = new ServerSocket(0)) {
            socket.setReuseAddress(true);
            return socket.getLocalPort();
        }
+1

btw, can we specify a port range ?

There are options to specify a range, but didn't look efficient to me.
org.springframework.util.SocketUtils#findAvailableTcpPort(int, int)
Its deprecated as well.

weizhouapache · 2023-12-06T13:06:08Z

plugins/backup/networker/src/test/java/org/apache/backup/networker/NetworkerClientTest.java

@@ -87,7 +99,7 @@ public void testListPolicies() {
                                "            \"comment\": \"-CSBKP-\",\n" +
                                "            \"links\": [\n" +
                                "                {\n" +
-                                "                    \"href\": \"http://localhost:9399/nwrestapi/v3/global/protectionpolicies/CSBRONZE\",\n" +
+                                "                    \"href\": \"http://localhost:9400/nwrestapi/v3/global/protectionpolicies/CSBRONZE\",\n" +


need to use url instead, the port might be different as 9400

private final String url = "http://localhost:" + port + "/nwrestapi/v3";

DaanHoogland · 2023-12-06T14:17:38Z

plugins/backup/networker/src/test/java/org/apache/backup/networker/NetworkerClientTest.java

@@ -60,6 +63,15 @@ public void setUp() throws Exception {
        client = new NetworkerClient(url, adminUsername, adminPassword, false, 60);
    }

+    private int getRandomPort() {


I think we should not do this. let's keep it deterministic.

vishesh92 · 2023-12-07T12:03:15Z

Let's fix the intermittent build failures for now. If random port is required, let's do that in a separate PR. Reverting the change for the random port for now and moving ahead with the fixed port.

shwstppr · 2023-12-07T12:21:55Z

@vishesh92 cc @DaanHoogland @kishankavala @weizhouapache I propose we move forward with this for now to address build failure and maybe create an issue or PR with more holistic changes to make a common util class to fetch an available port for different tests (in some range). Does that work?

weizhouapache · 2023-12-07T13:02:28Z

@vishesh92 cc @DaanHoogland @kishankavala @weizhouapache I propose we move forward with this for now to address build failure and maybe create an issue or PR with more holistic changes to make a common util class to fetch an available port for different tests (in some range). Does that work?

fine with me . let's use port 9400 for now.
if the port is used in a day in the future, we can use another port, it is not an issue.

Fix intermittent build failures

1449aa3

boring-cyborg bot added the component:backup label Dec 6, 2023

rohityadavcloud approved these changes Dec 6, 2023

View reviewed changes

DaanHoogland approved these changes Dec 6, 2023

View reviewed changes

shwstppr approved these changes Dec 6, 2023

View reviewed changes

shwstppr added this to the 4.19.0.0 milestone Dec 6, 2023

vishesh92 force-pushed the fix-intermittent-build-failures branch from 4b46737 to 1b4c9be Compare December 6, 2023 11:28

kishankavala reviewed Dec 6, 2023

View reviewed changes

weizhouapache reviewed Dec 6, 2023

View reviewed changes

DaanHoogland requested changes Dec 6, 2023

View reviewed changes

vishesh92 force-pushed the fix-intermittent-build-failures branch from 1b4c9be to 1449aa3 Compare December 7, 2023 12:02

shwstppr requested review from weizhouapache, kishankavala and DaanHoogland December 7, 2023 12:40

weizhouapache approved these changes Dec 7, 2023

View reviewed changes

weizhouapache merged commit 4e9c4a5 into apache:main Dec 7, 2023
48 of 49 checks passed

DaanHoogland deleted the fix-intermittent-build-failures branch December 8, 2023 13:37

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Dec 11, 2023

Fix intermittent build failures (apache#8312)

8951c77

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix intermittent build failures #8312

Fix intermittent build failures #8312

vishesh92 commented Dec 6, 2023 •

edited by shwstppr

rohityadavcloud commented Dec 6, 2023

DaanHoogland left a comment

DaanHoogland commented Dec 6, 2023

codecov bot commented Dec 6, 2023 •

edited

vishesh92 commented Dec 6, 2023

kishankavala commented Dec 6, 2023

DaanHoogland commented Dec 6, 2023

weizhouapache commented Dec 6, 2023

shwstppr left a comment

shwstppr commented Dec 6, 2023

vishesh92 commented Dec 6, 2023

vishesh92 commented Dec 6, 2023

kishankavala Dec 6, 2023

kishankavala Dec 6, 2023

vishesh92 Dec 6, 2023

DaanHoogland Dec 6, 2023

kishankavala commented Dec 6, 2023

weizhouapache Dec 6, 2023

DaanHoogland Dec 6, 2023

vishesh92 commented Dec 7, 2023 •

edited

shwstppr commented Dec 7, 2023

weizhouapache commented Dec 7, 2023

Fix intermittent build failures #8312

Fix intermittent build failures #8312

Conversation

vishesh92 commented Dec 6, 2023 • edited by shwstppr

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

rohityadavcloud commented Dec 6, 2023

DaanHoogland left a comment

Choose a reason for hiding this comment

DaanHoogland commented Dec 6, 2023

codecov bot commented Dec 6, 2023 • edited

Codecov Report

vishesh92 commented Dec 6, 2023

kishankavala commented Dec 6, 2023

DaanHoogland commented Dec 6, 2023

weizhouapache commented Dec 6, 2023

shwstppr left a comment

Choose a reason for hiding this comment

shwstppr commented Dec 6, 2023

vishesh92 commented Dec 6, 2023

vishesh92 commented Dec 6, 2023

kishankavala Dec 6, 2023

Choose a reason for hiding this comment

kishankavala Dec 6, 2023

Choose a reason for hiding this comment

vishesh92 Dec 6, 2023

Choose a reason for hiding this comment

DaanHoogland Dec 6, 2023

Choose a reason for hiding this comment

kishankavala commented Dec 6, 2023

weizhouapache Dec 6, 2023

Choose a reason for hiding this comment

DaanHoogland Dec 6, 2023

Choose a reason for hiding this comment

vishesh92 commented Dec 7, 2023 • edited

shwstppr commented Dec 7, 2023

weizhouapache commented Dec 7, 2023

vishesh92 commented Dec 6, 2023 •

edited by shwstppr

codecov bot commented Dec 6, 2023 •

edited

vishesh92 commented Dec 7, 2023 •

edited