weizhouapache commented on pull request #5839: URL: https://github.com/apache/cloudstack/pull/5839#issuecomment-1007316935
> > > clgtm, do we have a reason for this regression? > > > > > > @DaanHoogland > > the exception is thrown at https://github.com/apache/cloudstack/blob/main/engine/schema/src/main/java/com/cloud/network/dao/NetworkServiceMapDaoImpl.java#L125-L127 > > ``` > > if (ntwkSvc == null) { > > throw new UnsupportedServiceException("Service " + service.getName() + " is not supported in the network id=" + networkId); > > } > > ``` > > > > > > > > > > > > > > > > > > > > > > > > removing the lines might also fix the issue. but it has large impact. this PR has much smaller impact. > > I would agree @weizhouapache, I approve of this change and intuitively it is the best solution but still have some questions; > > * why is applyUserData called at all if it is not supported? and, > * is this a regression that might have other unforeseen consequences? > > It looks like it was introduced by [9c6b02f](https://github.com/apache/cloudstack/commit/9c6b02fd8b2282ceb80a1b9205871a4e93a00682). > @DaanHoogland when enable/disable static nat, the public IP of the vm needs to also updated in userdata server (=VR). by default, VM public IP is source nat IP. if static nat is enabled, VM public IP is the static nat IP. that's why we apply userdata of the vm/nic. you might have also noticed that there are already check below in code ``` if (element == null) { s_logger.error("Can't find network element for " + Service.UserData.getName() + " provider needed for UserData update"); ``` if element cannot be found, then do not apply userdata. but the exception is thrown in Dao (I think it is not a good idea) I have no idea if there are other issues caused by the code in Dao I pasted above. for enable/disable static nat, both should work no matter userdata is supported or not. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
