fix github actions build failure #1054

wind57 · 2022-07-26T12:48:57Z

No description provided.

ryanjbaxter · 2022-07-26T13:00:16Z

Thanks!

wind57 · 2022-07-26T19:58:08Z

note for myself, things to do here:

cache docker images via actions/upload-artifact@v2 and actions/download-artifact@v2
check if a certain test phase does not run tests in every module
why is the cache not re-used? It fails on a SocketTimeout about a certain junit-platform pom dependency, but shouldn't this one be part of the ~/m2 already? why is it trying to download it again?
check if tests per job_instance are actually run (I already did that when I made the PR, but somehow the logs are misleading in this direction)

wind57 · 2022-07-27T06:02:45Z

.github/workflows/maven.yaml

@@ -188,11 +184,17 @@ jobs:

          diff=$((right_bound - left_bound))
          sliced_array=("${CLASSNAMES[@]:$left_bound:$diff}")
-          TEST_ARG=$(echo $sliced_array | sed 's/ /,/g')
+          TEST_ARG=$(echo ${sliced_array[@]} | sed 's/ /,/g')


I had a bug here, I was only running the first test from a sliced_array, fixed.

wind57 · 2022-07-27T10:11:28Z

just found one more problem here with abstract classes... it is also present in circleci, so need to fix that also.

wind57 · 2022-07-27T13:37:02Z

...ud/kubernetes/client/config/KubernetesClientConfigMapIncludeProfileSpecificSourcesTests.java

@@ -1,107 +0,0 @@
-/*
- * Copyright 2013-2020 the original author or authors.


these two files were not used, I noticed this by accident

wind57 · 2022-07-27T13:40:06Z

.github/workflows/maven.yaml

+          # - find all tests
+          # - exclude Fabric8IstionIT
+          # - only take classes that have @Test inside them
+          # - ignore the ones that have 'abstract class'. we do this because otherwise we would pass


this small comment explains the problem, I noticed this by accident and could reproduce locally too... fixed via this logic.

wind57 · 2022-07-27T19:23:54Z

.circleci/config.yml

@@ -43,6 +43,11 @@ jobs:
            # needed for .withReuse(true) to work
            echo "testcontainers.reuse.enable=true" > ~/.testcontainers.properties
      - checkout
+      - restore_cache:


restore cache for tests

wind57 · 2022-07-27T20:17:14Z

this gets funner and funner, it seems with 653 tests we are hitting this issue

wind57 · 2022-08-05T12:32:18Z

I am still in the blur what is going on here. The test indeed fails (ActuatorRefreshKafkaIT), for me locally too, so I started to debug it a bit. The test itself does a few things:

starts configuration-watcher
starts configuration-watcher-it (the test itself)
starts an instance of kafka

So we start the application, then create a configmap with the same name and a proper label, this should trigger the informer (it does), in turn this will call applicationEventPublisher::publishEvent that is of type RefreshRemoteApplicationEvent (this happens too). In my understanding, at this point in time, I should see a message in springCloudBus topic, but when I have a lister:

bin/kafka-console-consumer.sh --topic springCloudBus --from-beginning --bootstrap-server localhost:9092

I don't see anything there. Weird. Still trying to figure out what is going on

wind57 · 2022-08-06T16:28:42Z

.../java/org/springframework/cloud/kubernetes/configuration/watcher/ActuatorRefreshKafkaIT.java

@@ -140,6 +141,8 @@ void after() throws Exception {
 		k8SUtils.waitForDeploymentToBeDeleted(SPRING_CLOUD_K8S_CONFIG_WATCHER_IT_DEPLOYMENT_NAME, NAMESPACE);
 	}

+	// TODO figure out why this one fails on bus-starter-4.0.0-SNAPSHOT
+	@Disabled


I am disabling it for now. I've spent a few good days trying to figure out what is going on here, but my limited understanding of bus/binders/functions/kafka/etc did not help. I'll try to spent some more time in the future, as I don't like disabling this at all :(

I've done the same test in 2.1.x branch and when I do bin/kafka-console-consumer.sh --topic springCloudBus --from-beginning --bootstrap-server localhost:9092, I do get a message in the kafka topic. interesting.

So you can reproduce this locally? What do I need to do to reproduce it?

It's going to be a bit of mouthful to explain how to reproduce this locally, but in the meanwhile I am on to something (I think). May be you know this off the top of your head.

So in 2.1.xwe get a Bean from BusRefreshAutoConfiguration called : RefreshListener:

@Bean @ConditionalOnProperty( value = {"spring.cloud.bus.refresh.enabled"}, matchIfMissing = true ) @ConditionalOnBean({ContextRefresher.class}) public RefreshListener refreshListener(ContextRefresher contextRefresher, ServiceMatcher serviceMatcher) { return new RefreshListener(contextRefresher, serviceMatcher); }

After this one is invoked, the message ends up in the kafka queue.

What is interested is that this Bean is present in main - I can get it from the context, but it's not present in the "applicationListeners" of the app. It's almost like there should be some code that registers it, but it's not. I'm sorry if this is obscure explanation, but I am still trying to figure out...

Weird, the autoconfiguration runs in main though?

Btw, it does not even matter if that name is wrong. You would still not see a message in the kafka queue, and that is the main problem. If you look at my example, I do not even have a consumer ( there is no spring-cloud-kubernetes-configuration-watcher-it deployed ).

I also do not know if you noticed, but the problem is for sure in a SNAPSHOT version. While this test was failing, the circleci one was passing - and we had a bug there where the cache was not updating with the latest snapshots.

I was talking to some people yesterday and there were some changes in Kafka recently that seems to correlate with the timeframe in which we saw these failures. I am looking into it some more. Hopefully will have an update next week.

OK so I finally nailed down that was happening after I was able to reproduce the problem using a simple Spring Cloud Bus + Kafka app. This exception was being thrown when we called triggerRefresh

java.lang.NoSuchMethodError: 'org.springframework.util.concurrent.ListenableFuture org.springframework.kafka.core.KafkaTemplate.send(org.apache.kafka.clients.producer.ProducerRecord)' at org.springframework.integration.kafka.outbound.KafkaProducerMessageHandler.handleRequestMessage(KafkaProducerMessageHandler.java:513) ~[spring-integration-kafka-6.0.0-M4.jar:6.0.0-M4]

The reason why we didn't see it is because the ExecutorService was swallowing it 🤦‍♂️.

I added a try/catch with a log statement around that call so we can catch anything like this in the future.

spring-cloud-kubernetes/spring-cloud-kubernetes-controllers/spring-cloud-kubernetes-configuration-watcher/src/main/java/org/springframework/cloud/kubernetes/configuration/watcher/ConfigMapWatcherChangeDetector.java

Line 69 in 17b61c2

triggerRefresh(configMap).subscribe();

The reason for the NoSuchMethodError was because there was a chance in Spring Core to ListenableFuture but the Spring Boot snapshots were using Spring Integration Milestone 4 which didn't have the corresponding change to support the change to ListenableFuture. I got the Boot team to move Spring Integration to snapshots and that fixed the underlying problem.

So we should be able to remove the @Ignore from the test now and that build should pass.

NICE!! Indeed it builds just fine now :) take a look when you have time

test

9b3bd26

spring-cloud-issues added the waiting-for-triage label Jul 26, 2022

wind57 added 4 commits July 26, 2022 16:52

add timeout

59c5995

trigger

73281a2

add timeout to 60 min

266890d

test

67b6a34

wind57 added 3 commits July 26, 2022 23:12

test

7a15fd9

test

e5d45f8

fix what tests each instance is running

fe8b32d

wind57 commented Jul 27, 2022

View reviewed changes

github fix for abstract tests

e19a0c4

wind57 commented Jul 27, 2022

View reviewed changes

wind57 added 2 commits July 27, 2022 16:38

remove file

9d37ad5

removed echo

e98200f

wind57 commented Jul 27, 2022

View reviewed changes

wind57 added 6 commits July 27, 2022 16:53

test circleci

230f675

escape

6f6e90d

single?

8f69377

test

00f0842

bigger timeout

1697e0b

add a bit more debugging

b4de760

wind57 commented Jul 27, 2022

View reviewed changes

fix circleci

426ed3f

wind57 added 3 commits July 28, 2022 07:26

split in half

ae804f6

test

be919bb

test

bc58f7e

wind57 added 20 commits August 4, 2022 09:01

increaase timeout

a7a37ad

increaase timeout

1407fc5

test

2ed5072

test

0f4c477

test

6b6fe6f

test

dcde155

no return

2a1ef4a

test

b507c5e

test

fdf148a

test

de31f65

test

3a3522d

test

3d52745

test

249ed84

test

df27866

test

a201e93

test

9340505

test

cc6bde5

test

0032a14

test

6dc5c42

test

bebdb7c

disable test

cad4be0

wind57 commented Aug 6, 2022

View reviewed changes

wind57 requested a review from ryanjbaxter August 6, 2022 17:36

wind57 added 3 commits August 16, 2022 22:05

pom

39c3c68

Merge branch 'main' into fix-build-on-github

28f0618

enable test back

1479efd

ryanjbaxter added task and removed waiting-for-triage labels Aug 17, 2022

ryanjbaxter merged commit d817507 into spring-cloud:main Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix github actions build failure #1054

fix github actions build failure #1054

wind57 commented Jul 26, 2022

ryanjbaxter commented Jul 26, 2022

wind57 commented Jul 26, 2022 •

edited

Loading

wind57 Jul 27, 2022

wind57 commented Jul 27, 2022

wind57 Jul 27, 2022

wind57 Jul 27, 2022 •

edited

Loading

wind57 Jul 27, 2022

wind57 commented Jul 27, 2022 •

edited

Loading

wind57 commented Aug 5, 2022 •

edited

Loading

wind57 Aug 6, 2022

wind57 Aug 8, 2022

ryanjbaxter Aug 9, 2022

wind57 Aug 9, 2022

ryanjbaxter Aug 9, 2022

wind57 Aug 12, 2022

wind57 Aug 12, 2022

ryanjbaxter Aug 12, 2022

ryanjbaxter Aug 16, 2022

wind57 Aug 17, 2022

		@@ -1,107 +0,0 @@
		/*
		* Copyright 2013-2020 the original author or authors.

fix github actions build failure #1054

fix github actions build failure #1054

Conversation

wind57 commented Jul 26, 2022

ryanjbaxter commented Jul 26, 2022

wind57 commented Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

wind57 commented Jul 27, 2022

Choose a reason for hiding this comment

wind57 Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wind57 commented Jul 27, 2022 • edited Loading

wind57 commented Aug 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wind57 commented Jul 26, 2022 •

edited

Loading

wind57 Jul 27, 2022 •

edited

Loading

wind57 commented Jul 27, 2022 •

edited

Loading

wind57 commented Aug 5, 2022 •

edited

Loading