Re: [PR] fix error during kubernetes cluster creation [cloudstack]
weizhouapache commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2344499601 @GaOrtiga Can you update the title with short summay of the issue ? -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2344431394 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11098 -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2344308407 @GaOrtiga a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
GaOrtiga commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2344305592 @blueorangutan package -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2288871847 Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10658 -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2287107446 Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10641 -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2286990675 @JoaoJandre a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
JoaoJandre commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2286987990 @blueorangutan package -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
DaanHoogland commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2238619491 @borisstoyanov , @FelipeM525 tested (https://github.com/apache/cloudstack/pull/9223#pullrequestreview-2180689837), do you still want to test this as well? -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2238095955 [SF] Trillian test result (tid-10912) Environment: kvm-alma8 (x2), Advanced Networking with Mgmt server a8 Total time taken: 52725 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9223-t10912-kvm-alma8.zip Smoke tests completed. 135 look OK, 2 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_06_purge_expunged_vm_background_task | `Failure` | 339.14 | test_purge_expunged_vms.py test_03_secured_to_nonsecured_vm_migration | `Error` | 396.95 | test_vm_life_cycle.py -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2234307073 [SF] Trillian test result (tid-10884) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 51258 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9223-t10884-kvm-centos7.zip Smoke tests completed. 129 look OK, 8 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_role_account_acls_multiple_mgmt_servers | `Error` | 2.15 | test_dynamicroles.py test_query_async_job_result | `Error` | 102.01 | test_async_job.py test_revoke_certificate | `Error` | 0.02 | test_certauthority_root.py test_configure_ha_provider_invalid | `Error` | 0.01 | test_hostha_simulator.py test_configure_ha_provider_valid | `Error` | 0.04 | test_hostha_simulator.py test_ha_configure_enabledisable_across_clusterzones | `Error` | 0.01 | test_hostha_simulator.py test_ha_disable_feature_invalid | `Error` | 0.01 | test_hostha_simulator.py test_ha_enable_feature_invalid | `Error` | 0.01 | test_hostha_simulator.py test_ha_list_providers | `Error` | 0.01 | test_hostha_simulator.py test_ha_multiple_mgmt_server_ownership | `Error` | 0.01 | test_hostha_simulator.py test_ha_verify_fsm_available | `Error` | 0.01 | test_hostha_simulator.py test_ha_verify_fsm_degraded | `Error` | 0.01 | test_hostha_simulator.py test_ha_verify_fsm_fenced | `Error` | 0.01 | test_hostha_simulator.py test_ha_verify_fsm_recovering | `Error` | 0.01 | test_hostha_simulator.py test_hostha_configure_default_driver | `Error` | 0.01 | test_hostha_simulator.py test_hostha_configure_invalid_provider | `Error` | 0.01 | test_hostha_simulator.py test_hostha_disable_feature_valid | `Error` | 0.01 | test_hostha_simulator.py test_hostha_enable_feature_valid | `Error` | 0.01 | test_hostha_simulator.py test_hostha_enable_feature_without_setting_provider | `Error` | 0.01 | test_hostha_simulator.py test_list_ha_for_host | `Error` | 0.01 | test_hostha_simulator.py test_list_ha_for_host_invalid | `Error` | 0.01 | test_hostha_simulator.py test_list_ha_for_host_valid | `Error` | 0.01 | test_hostha_simulator.py test_01_host_ping_on_alert | `Error` | 0.07 | test_host_ping.py test_01_host_ping_on_alert | `Error` | 0.07 | test_host_ping.py test_01_browser_migrate_template | `Error` | 15.30 | test_image_store_object_migration.py test_06_purge_expunged_vm_background_task | `Failure` | 340.83 | test_purge_expunged_vms.py test_hostha_enable_ha_when_host_disabled | `Error` | 5.79 | test_hostha_kvm.py test_hostha_enable_ha_when_host_in_maintenance | `Error` | 305.91 | test_hostha_kvm.py -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2232529757 @DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
DaanHoogland commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2232526671 @blueorangutan test -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2231732568 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10382 -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2231602229 @DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
DaanHoogland commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2231599185 @blueorangutan package -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2165712871 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9929 -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
borisstoyanov commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2165264616 @blueorangutan package -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2165266139 @borisstoyanov a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2165501603 @borisstoyanov a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
borisstoyanov commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2165496427 thanks @GaOrtiga @blueorangutan package -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
blueorangutan commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2165365606 Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9927 -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
weizhouapache commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1636927477 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java: ## @@ -378,7 +378,7 @@ public VMTemplateVO getKubernetesServiceTemplate(DataCenter dataCenter, Hypervis } protected void validateIsolatedNetworkIpRules(long ipId, FirewallRule.Purpose purpose, Network network, int clusterTotalNodeCount) { -List rules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose); +List rules = firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(ipId, purpose, "tcp"); Review Comment: 'tcp' can be replaced with NetUtils.TCP_PROTO -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
GaOrtiga commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1636740222 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java: ## @@ -379,8 +380,8 @@ public VMTemplateVO getKubernetesServiceTemplate(DataCenter dataCenter, Hypervis protected void validateIsolatedNetworkIpRules(long ipId, FirewallRule.Purpose purpose, Network network, int clusterTotalNodeCount) { List rules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose); for (FirewallRuleVO rule : rules) { -Integer startPort = rule.getSourcePortStart(); -Integer endPort = rule.getSourcePortEnd(); +int startPort = ObjectUtils.defaultIfNull(rule.getSourcePortStart(), 1); +int endPort = ObjectUtils.defaultIfNull(rule.getSourcePortEnd(), KubernetesClusterActionWorker.MAX_PORT); Review Comment: @weizhouapache Thank you for the clarification. Yes, I believe you are correct, they should be skipped. I can make the adjustment in this PR, no need for a new one. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
GaOrtiga commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1636740222 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java: ## @@ -379,8 +380,8 @@ public VMTemplateVO getKubernetesServiceTemplate(DataCenter dataCenter, Hypervis protected void validateIsolatedNetworkIpRules(long ipId, FirewallRule.Purpose purpose, Network network, int clusterTotalNodeCount) { List rules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose); for (FirewallRuleVO rule : rules) { -Integer startPort = rule.getSourcePortStart(); -Integer endPort = rule.getSourcePortEnd(); +int startPort = ObjectUtils.defaultIfNull(rule.getSourcePortStart(), 1); +int endPort = ObjectUtils.defaultIfNull(rule.getSourcePortEnd(), KubernetesClusterActionWorker.MAX_PORT); Review Comment: @weizhouapache Thank you for the clarification. Yes, I believe you are correct, they should be skipped. I can make this adjustment in this PR, no need for a new one. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
weizhouapache commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1636554494 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java: ## @@ -379,8 +380,8 @@ public VMTemplateVO getKubernetesServiceTemplate(DataCenter dataCenter, Hypervis protected void validateIsolatedNetworkIpRules(long ipId, FirewallRule.Purpose purpose, Network network, int clusterTotalNodeCount) { List rules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose); for (FirewallRuleVO rule : rules) { -Integer startPort = rule.getSourcePortStart(); -Integer endPort = rule.getSourcePortEnd(); +int startPort = ObjectUtils.defaultIfNull(rule.getSourcePortStart(), 1); +int endPort = ObjectUtils.defaultIfNull(rule.getSourcePortEnd(), KubernetesClusterActionWorker.MAX_PORT); Review Comment: @GaOrtiga this code snippet checks if there are firewall rules conflicting with the kubernetes API (tcp/6443) and node ssh (tcp/, tcp/2223, etc). IMHO, if firewall rules do not use TCP protocol, the ports check can be skipped. for instance udp/6443 has no conflicts with tcp/6443. I can create another pr for it. never mind. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
weizhouapache commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1636548382 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java: ## @@ -105,6 +105,7 @@ public class KubernetesClusterActionWorker { public static final int DEFAULT_SSH_PORT = 22; public static final int CLUSTER_NODES_DEFAULT_START_SSH_PORT = ; public static final int CLUSTER_NODES_DEFAULT_SSH_PORT_SG = DEFAULT_SSH_PORT; +public static final int MAX_PORT = 65535; Review Comment: this could be moved to NetUtils -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
codecov[bot] commented on PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#issuecomment-2163113513 ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/9223?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 4.20%. Comparing base [(`0c61531`)](https://app.codecov.io/gh/apache/cloudstack/commit/0c615312f95aa3c0a23ad5baa7e3ad2b308133d7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`a4c9718`)](https://app.codecov.io/gh/apache/cloudstack/commit/a4c9718c7dba8734665e84e0edb5491006ecfb1a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). > Report is 28 commits behind head on main. Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #9223 +/- ## - Coverage 15.28% 4.20% -11.08% Files 5424 368 -5056 Lines474331 30161 -444170 Branches 602415307-54934 - Hits 724961269-71227 + Misses 393777 28748 -365029 + Partials 8058 144 -7914 ``` | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/9223/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/9223/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.20% <ø> (-0.05%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/9223/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/9223?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
DaanHoogland commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1636521466 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java: ## @@ -105,6 +105,7 @@ public class KubernetesClusterActionWorker { public static final int DEFAULT_SSH_PORT = 22; public static final int CLUSTER_NODES_DEFAULT_START_SSH_PORT = ; public static final int CLUSTER_NODES_DEFAULT_SSH_PORT_SG = DEFAULT_SSH_PORT; +public static final int MAX_PORT = 65535; Review Comment: ok, never mind. It seemed such a standard value. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
GaOrtiga commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1636324203 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java: ## @@ -105,6 +105,7 @@ public class KubernetesClusterActionWorker { public static final int DEFAULT_SSH_PORT = 22; public static final int CLUSTER_NODES_DEFAULT_START_SSH_PORT = ; public static final int CLUSTER_NODES_DEFAULT_SSH_PORT_SG = DEFAULT_SSH_PORT; +public static final int MAX_PORT = 65535; Review Comment: I have searched for it, without success. However, if someone else knows where to find this constant, I can swap it. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
GaOrtiga commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1636324016 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java: ## @@ -379,8 +380,8 @@ public VMTemplateVO getKubernetesServiceTemplate(DataCenter dataCenter, Hypervis protected void validateIsolatedNetworkIpRules(long ipId, FirewallRule.Purpose purpose, Network network, int clusterTotalNodeCount) { List rules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose); for (FirewallRuleVO rule : rules) { -Integer startPort = rule.getSourcePortStart(); -Integer endPort = rule.getSourcePortEnd(); +int startPort = ObjectUtils.defaultIfNull(rule.getSourcePortStart(), 1); +int endPort = ObjectUtils.defaultIfNull(rule.getSourcePortEnd(), KubernetesClusterActionWorker.MAX_PORT); Review Comment: Thank you @weizhouapache. Can you further explain this comment? As it stands this PR maintains the current behaviour of validating rule conflicts for rules with every protocol, without any differentiation between TCP, UDP and ICMP. I have, however, run tests using all three protocols, and all of them shared the same results. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
DaanHoogland commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1634985937 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java: ## @@ -105,6 +105,7 @@ public class KubernetesClusterActionWorker { public static final int DEFAULT_SSH_PORT = 22; public static final int CLUSTER_NODES_DEFAULT_START_SSH_PORT = ; public static final int CLUSTER_NODES_DEFAULT_SSH_PORT_SG = DEFAULT_SSH_PORT; +public static final int MAX_PORT = 65535; Review Comment: is there a java constant (in java.net or java.lang?) -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix error during kubernetes cluster creation [cloudstack]
weizhouapache commented on code in PR #9223: URL: https://github.com/apache/cloudstack/pull/9223#discussion_r1634976649 ## plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java: ## @@ -379,8 +380,8 @@ public VMTemplateVO getKubernetesServiceTemplate(DataCenter dataCenter, Hypervis protected void validateIsolatedNetworkIpRules(long ipId, FirewallRule.Purpose purpose, Network network, int clusterTotalNodeCount) { List rules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose); for (FirewallRuleVO rule : rules) { -Integer startPort = rule.getSourcePortStart(); -Integer endPort = rule.getSourcePortEnd(); +int startPort = ObjectUtils.defaultIfNull(rule.getSourcePortStart(), 1); +int endPort = ObjectUtils.defaultIfNull(rule.getSourcePortEnd(), KubernetesClusterActionWorker.MAX_PORT); Review Comment: good catch @GaOrtiga should firewall with other protocols (not TCP) be skipped ? -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] fix error during kubernetes cluster creation [cloudstack]
GaOrtiga opened a new pull request, #9223: URL: https://github.com/apache/cloudstack/pull/9223 ### Description During the creation of firewall rules, if one of the limits for the ports is not informed, it is saved in the database as Null, indicating that there is no starting/ending limit. while creating a Kubernetes cluster, if the selected network has a rule that contains ports saved as null, an error is thrown, stopping the execution of the process. This behaviour has been fixed, making it so that any port saved as null is regarded as being being on its respective limit (1 for start ports and 65535 for end ports) during the creation of a Kubernetes cluster. ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [X] 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 Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [X] Minor - [ ] Trivial ### Screenshots (if appropriate): ### How Has This Been Tested? I created a rule in an existing network, with null starting port and ending on port 10, making it so it should not conflict with any of the ports required by the Kubernetes cluster. Before applying the changes, an error would be thrown. I repeated the process after applying the changes and the cluster was successfully created. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org