[ISSUE#8653]Fix health check bug

naming/src/main/java/com/alibaba/nacos/naming/core/v2/service/impl/PersistentClientOperationServiceImpl.java

@@ -118,6 +118,31 @@ public void registerInstance(Service service, Instance instance, String clientId
  }
  }
 
+ /**
+ * update persistent instance.
+ */
+ public void updateInstance(Service service, Instance instance, String clientId) {
+ Service singleton = ServiceManager.getInstance().getSingleton(service);
+ if (singleton.isEphemeral()) {
+ throw new NacosRuntimeException(NacosException.INVALID_PARAM,
+ String.format("Current service %s is ephemeral service, can't update persistent instance.",
+ singleton.getGroupedServiceName()));
+ }
+ final PersistentClientOperationServiceImpl.InstanceStoreRequest request = new PersistentClientOperationServiceImpl.InstanceStoreRequest();
+ request.setService(service);
+ request.setInstance(instance);
+ request.setClientId(clientId);
+ final WriteRequest writeRequest = WriteRequest.newBuilder()
+ .setGroup(group())
+ .setData(ByteString.copyFrom(serializer.serialize(request))).setOperation(DataOperation.CHANGE.name())
+ .build();
+ try {
+ protocol.write(writeRequest);
+ } catch (Exception e) {
+ throw new NacosRuntimeException(NacosException.SERVER_ERROR, e);
+ }
+ }
+
  @Override
  public void batchRegisterInstance(Service service, List<Instance> instances, String clientId) {
  //TODO PersistentClientOperationServiceImpl Nacos batchRegister
@@ -170,6 +195,12 @@ public Response onApply(WriteRequest request) {
  case DELETE:
  onInstanceDeregister(instanceRequest.service, instanceRequest.getClientId());
  break;
+ case CHANGE:
+ if (instanceAndServiceExist(instanceRequest)) {
+ onInstanceRegister(instanceRequest.service, instanceRequest.instance,
+ instanceRequest.getClientId());
+ }
+ break;
  default:
  return Response.newBuilder().setSuccess(false).setErrMsg("unsupport operation : " + operation)
  .build();
@@ -180,6 +211,11 @@ public Response onApply(WriteRequest request) {
  }
  }
 
+ private boolean instanceAndServiceExist(InstanceStoreRequest instanceRequest) {
+ return clientManager.contains(instanceRequest.getClientId()) && clientManager.getClient(
+ instanceRequest.getClientId()).getAllPublishedService().contains(instanceRequest.service);
+ }
+
  private void onInstanceRegister(Service service, Instance instance, String clientId) {
  Service singleton = ServiceManager.getInstance().getSingleton(service);
  if (!clientManager.contains(clientId)) {

naming/src/main/java/com/alibaba/nacos/naming/healthcheck/v2/PersistentHealthStatusSynchronizer.java

@@ -43,6 +43,6 @@ public void instanceHealthStatusChange(boolean isHealthy, Client client, Service
  InstancePublishInfo instance) {
  Instance updateInstance = InstanceUtil.parseToApiInstance(service, instance);
  updateInstance.setHealthy(isHealthy);
- persistentClientOperationService.registerInstance(service, updateInstance, client.getClientId());
+ persistentClientOperationService.updateInstance(service, updateInstance, client.getClientId());
  }
 }

naming/src/test/java/com/alibaba/nacos/naming/core/ClusterTest.java

@@ -18,6 +18,7 @@
 
 import com.alibaba.nacos.api.naming.pojo.healthcheck.impl.Http;
 import com.alibaba.nacos.common.utils.JacksonUtils;
+import com.alibaba.nacos.sys.env.EnvUtil;
 import com.alibaba.nacos.sys.utils.ApplicationUtils;
 import com.alibaba.nacos.naming.misc.SwitchDomain;
 import com.alibaba.nacos.naming.misc.SwitchDomain.TcpHealthParams;
@@ -29,6 +30,7 @@
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.mock.env.MockEnvironment;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -53,6 +55,7 @@ public class ClusterTest {
 
  @Before
  public void before() {
+ EnvUtil.setEnvironment(new MockEnvironment());
  ApplicationUtils.injectContext(context);
  when(context.getBean(SwitchDomain.class)).thenReturn(switchDomain);
  when(switchDomain.getTcpHealthParams()).thenReturn(new TcpHealthParams());

naming/src/test/java/com/alibaba/nacos/naming/core/v2/service/impl/PersistentClientOperationServiceImplTest.java

@@ -41,6 +41,7 @@
 import org.springframework.context.ConfigurableApplicationContext;
 
 import java.lang.reflect.Field;
+import java.util.Collections;
 
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.times;
@@ -77,7 +78,10 @@ public class PersistentClientOperationServiceImplTest {
 
  @Mock
  private Serializer serializer;
-
+
+ @Mock
+ private IpPortBasedClient ipPortBasedClient;
+
  @Before
  public void setUp() throws Exception {
  when(service.getNamespace()).thenReturn("n");
@@ -90,6 +94,7 @@ public void setUp() throws Exception {
  serializerField.setAccessible(true);
  persistentClientOperationServiceImpl = new PersistentClientOperationServiceImpl(clientManager);
  serializerField.set(persistentClientOperationServiceImpl, serializer);
+
  }
 
  @Test(expected = NacosRuntimeException.class)
@@ -109,7 +114,16 @@ public void testRegisterAndDeregisterInstance() throws Exception {
  persistentClientOperationServiceImpl.deregisterInstance(service, instance, clientId);
  verify(cpProtocol, times(2)).write(any(WriteRequest.class));
  }
-
+
+ @Test
+ public void updateInstance() throws Exception {
+ Field clientManagerField = PersistentClientOperationServiceImpl.class.getDeclaredField("clientManager");
+ clientManagerField.setAccessible(true);
+ // Test register instance
+ persistentClientOperationServiceImpl.updateInstance(service, instance, clientId);
+ verify(cpProtocol).write(any(WriteRequest.class));
+ }
+
  @Test(expected = UnsupportedOperationException.class)
  public void testSubscribeService() {
  persistentClientOperationServiceImpl.subscribeService(service, subscriber, clientId);
@@ -145,7 +159,7 @@ public void testOnApply() {
  .build();
  Response response = persistentClientOperationServiceImpl.onApply(writeRequest);
  Assert.assertTrue(response.getSuccess());
-
+  Assert.assertTrue(ServiceManager.getInstance().containSingleton(service1));
  writeRequest = WriteRequest.newBuilder()
  .setOperation(DataOperation.DELETE.name())
  .build();
@@ -158,5 +172,39 @@ public void testOnApply() {
  .build();
  response = persistentClientOperationServiceImpl.onApply(writeRequest);
  Assert.assertFalse(response.getSuccess());
+
+ writeRequest = WriteRequest.newBuilder()
+ .setOperation(DataOperation.CHANGE.name())
+ .build();
+ response = persistentClientOperationServiceImpl.onApply(writeRequest);
+ Assert.assertTrue(response.getSuccess());
+ Assert.assertFalse(ServiceManager.getInstance().containSingleton(service1));
+ }
+
+ @Test
+ public void testOnApplyChange() {
+ PersistentClientOperationServiceImpl.InstanceStoreRequest request = new PersistentClientOperationServiceImpl.InstanceStoreRequest();
+ Service service1 = Service.newService("A", "B", "C");
+ request.setService(service1);
+ request.setClientId("xxxx");
+ request.setInstance(new Instance());
+ Mockito.when(serializer.deserialize(Mockito.any())).thenReturn(request);
+ Mockito.when(clientManager.contains(Mockito.anyString())).thenReturn(true);
+ when(clientManager.getClient(Mockito.anyString())).thenReturn(ipPortBasedClient);
+ when(ipPortBasedClient.getAllPublishedService()).thenReturn(Collections.singletonList(service1));
+ WriteRequest writeRequest = WriteRequest.newBuilder()
+ .setOperation(DataOperation.ADD.name())
+ .build();
+ writeRequest = WriteRequest.newBuilder()
+ .setOperation(DataOperation.ADD.name())
+ .build();
+ Response response = persistentClientOperationServiceImpl.onApply(writeRequest);
+ Assert.assertTrue(response.getSuccess());
+ writeRequest = WriteRequest.newBuilder()
+ .setOperation(DataOperation.CHANGE.name())
+ .build();
+ response = persistentClientOperationServiceImpl.onApply(writeRequest);
+ Assert.assertTrue(response.getSuccess());
+ Assert.assertTrue(ServiceManager.getInstance().containSingleton(service1));
  }
 }

naming/src/test/java/com/alibaba/nacos/naming/healthcheck/v2/PersistentHealthStatusSynchronizerTest.java

@@ -50,6 +50,6 @@ public void testInstanceHealthStatusChange() {
  updateInstance.setHealthy(true);
 
  verify(client).getClientId();
- verify(persistentClientOperationService).registerInstance(service, updateInstance, client.getClientId());
+ verify(persistentClientOperationService).updateInstance(service, updateInstance, client.getClientId());
  }
 }