diff --git a/devstack/plugin.sh b/devstack/plugin.sh index e7d0c7059..828f8980a 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -284,6 +284,9 @@ function configure_neutron_defaults { sg_ids=$(echo $(neutron security-group-list \ --project-id "$project_id" -c id -f value) | tr ' ' ',') + ext_svc_net_id="$(neutron net-show -c id -f value \ + "${KURYR_NEUTRON_DEFAULT_EXT_SVC_NET}")" + ext_svc_subnet_id="$(neutron subnet-show -c id -f value \ "${KURYR_NEUTRON_DEFAULT_EXT_SVC_SUBNET}")" @@ -340,7 +343,7 @@ function configure_neutron_defaults { if [ -n "$OVS_BRIDGE" ]; then iniset "$KURYR_CONFIG" neutron_defaults ovs_bridge "$OVS_BRIDGE" fi - iniset "$KURYR_CONFIG" neutron_defaults external_svc_subnet "$ext_svc_subnet_id" + iniset "$KURYR_CONFIG" neutron_defaults external_svc_net "$ext_svc_net_id" iniset "$KURYR_CONFIG" octavia_defaults member_mode "$KURYR_K8S_OCTAVIA_MEMBER_MODE" } diff --git a/devstack/settings b/devstack/settings index 3d4e56468..fa8574f24 100644 --- a/devstack/settings +++ b/devstack/settings @@ -18,6 +18,7 @@ KURYR_NEUTRON_DEFAULT_POD_SUBNET=${KURYR_NEUTRON_DEFAULT_POD_SUBNET:-k8s-pod-sub KURYR_NEUTRON_DEFAULT_SERVICE_SUBNET=${KURYR_NEUTRON_DEFAULT_SERVICE_SUBNET:-k8s-service-subnet} KURYR_NEUTRON_DEFAULT_SUBNETPOOL_ID=${KURYR_NEUTRON_DEFAULT_SUBNETPOOL_ID:-} KURYR_NEUTRON_DEFAULT_ROUTER=${KURYR_NEUTRON_DEFAULT_ROUTER:-} +KURYR_NEUTRON_DEFAULT_EXT_SVC_NET=${KURYR_NEUTRON_DEFAULT_EXT_SVC_NET:-public} KURYR_NEUTRON_DEFAULT_EXT_SVC_SUBNET=${KURYR_NEUTRON_DEFAULT_EXT_SVC_SUBNET:-public-subnet} # Etcd diff --git a/doc/source/installation/services.rst b/doc/source/installation/services.rst index 4e1ebc5aa..a914f49d8 100644 --- a/doc/source/installation/services.rst +++ b/doc/source/installation/services.rst @@ -412,10 +412,13 @@ The services and pods subnets should be created. A. Create an external/provider network B. Create subnet/pool range of external CIDR C. Connect external subnet to kuryr-kubernetes router - D. Configure Kuryr.conf public ip subnet to point to the external subnet:: + D. Configure external network details in Kuryr.conf as follows:: [neutron_defaults] - external_svc_subnet= external_subnet_id + external_svc_net= + # 'external_svc_subnet' field is optional, set this field in case + # multiple subnets attached to 'external_svc_net' + external_svc_subnet= From this point for each K8s service of type=LoadBalancer and in which 'load-balancer-ip' is not specified, an external IP from diff --git a/kuryr_kubernetes/config.py b/kuryr_kubernetes/config.py index 463e03b0c..4bd3a1127 100644 --- a/kuryr_kubernetes/config.py +++ b/kuryr_kubernetes/config.py @@ -153,8 +153,11 @@ neutron_defaults = [ sample_default="br-int"), cfg.StrOpt('service_subnet', help=_("Default Neutron subnet ID for Kubernetes services")), + cfg.StrOpt('external_svc_net', + help=_("Default external network ID for Kubernetes services")), cfg.StrOpt('external_svc_subnet', - help=_("Default external subnet for Kubernetes services")), + help=_("Optional external subnet ID for Kubernetes services"), + default=None), ] octavia_defaults = [ diff --git a/kuryr_kubernetes/controller/drivers/lb_public_ip.py b/kuryr_kubernetes/controller/drivers/lb_public_ip.py index 386e0c4a3..3fc32d8c5 100644 --- a/kuryr_kubernetes/controller/drivers/lb_public_ip.py +++ b/kuryr_kubernetes/controller/drivers/lb_public_ip.py @@ -12,8 +12,6 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -from kuryr.lib import exceptions as kl_exc -from kuryr_kubernetes import clients from kuryr_kubernetes import config from kuryr_kubernetes.controller.drivers import base from kuryr_kubernetes.controller.drivers import public_ip @@ -63,27 +61,17 @@ class FloatingIpServicePubIPDriver(base.ServicePubIpDriver): else: LOG.debug("Trying to allocate public ip from pool") - # get public subnet id from kuryr.conf - external_svc_subnet = config.CONF.neutron_defaults.external_svc_subnet - if not external_svc_subnet: - raise cfg.RequiredOptError('external_svc_subnet', + # get public network/subnet ids from kuryr.conf + public_network_id = config.CONF.neutron_defaults.external_svc_net + public_subnet_id = config.CONF.neutron_defaults.external_svc_subnet + if not public_network_id: + raise cfg.RequiredOptError('external_svc_net', cfg.OptGroup('neutron_defaults')) - neutron = clients.get_neutron_client() - n_subnet = neutron.show_subnet(external_svc_subnet).get('subnet') - if not n_subnet: - LOG.error( - "No subnet found for external_svc_subnet=%s", - external_svc_subnet) - raise kl_exc.NoResourceException - - public_network_id = n_subnet['network_id'] - - res_id, alloc_ip_addr = (self._drv_pub_ip.allocate_ip - (public_network_id, - external_svc_subnet, - project_id, - 'kuryr_lb')) + res_id, alloc_ip_addr = ( + self._drv_pub_ip.allocate_ip(public_network_id, project_id, + pub_subnet_id=public_subnet_id, + description='kuryr_lb')) service_pub_ip_info = obj_lbaas.LBaaSPubIp(ip_id=res_id, ip_addr=alloc_ip_addr, alloc_method='pool') diff --git a/kuryr_kubernetes/controller/drivers/public_ip.py b/kuryr_kubernetes/controller/drivers/public_ip.py index 690ae61d3..e92541598 100644 --- a/kuryr_kubernetes/controller/drivers/public_ip.py +++ b/kuryr_kubernetes/controller/drivers/public_ip.py @@ -36,13 +36,14 @@ class BasePubIpDriver(object): raise NotImplementedError() @abc.abstractmethod - def allocate_ip(self, pub_net_id, pub_subnet_id, project_id, description): + def allocate_ip(self, pub_net_id, project_id, pub_subnet_id=None, + description=None): """allocate ip address from public network id :param pub_net_id: public network id - :param pub_subnet_id: public subnet id :param project_id: - :param description: string describing request + :param pub_subnet_id: public subnet id (Optional) + :param description: string describing request (Optional) :returns res_id , ip_addr :res_id - resource id :ip_addr - ip aaddress @@ -100,19 +101,23 @@ class FipPubIpDriver(BasePubIpDriver): LOG.error("Invalid parameter ip_addr=%s", ip_addr) return None - def allocate_ip(self, pub_net_id, pub_subnet_id, project_id, description): - + def allocate_ip(self, pub_net_id, project_id, pub_subnet_id=None, + description=None): neutron = clients.get_neutron_client() + request = {'floatingip': { + 'tenant_id': project_id, + 'project_id': project_id, + 'floating_network_id': pub_net_id}} + + if pub_subnet_id is not None: + request['floatingip']['subnet_id'] = pub_subnet_id + if description is not None: + request['floatingip']['description'] = description + try: - response = neutron.create_floatingip({'floatingip': { - 'tenant_id': project_id, - 'project_id': project_id, - 'floating_network_id': pub_net_id, - 'subnet_id': pub_subnet_id, - 'description': description}}) + response = neutron.create_floatingip(request) except n_exc.NeutronClientException as ex: - LOG.error("Failed to create floating IP - subnetid=%s ", - pub_subnet_id) + LOG.error("Failed to create floating IP - netid=%s ", pub_net_id) raise ex return response['floatingip']['id'], response[ 'floatingip']['floating_ip_address'] diff --git a/kuryr_kubernetes/controller/handlers/lbaas.py b/kuryr_kubernetes/controller/handlers/lbaas.py index 887a7a14a..64cf06038 100644 --- a/kuryr_kubernetes/controller/handlers/lbaas.py +++ b/kuryr_kubernetes/controller/handlers/lbaas.py @@ -272,8 +272,11 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler): return sorted(ep_ports) == sorted(spec_ports) def _has_pods(self, endpoints): + ep_subsets = endpoints.get('subsets', []) + if not ep_subsets: + return False return any(True - for subset in endpoints.get('subsets', []) + for subset in ep_subsets for address in subset.get('addresses', []) if address.get('targetRef', {}).get('kind') == 'Pod') diff --git a/kuryr_kubernetes/handlers/retry.py b/kuryr_kubernetes/handlers/retry.py index 38dedf28f..a4671a14e 100644 --- a/kuryr_kubernetes/handlers/retry.py +++ b/kuryr_kubernetes/handlers/retry.py @@ -70,6 +70,7 @@ class Retry(base.EventHandler): except Exception: LOG.debug('Report handler unhealthy %s', self._handler) self._handler.set_health_status(healthy=False) + raise def _sleep(self, deadline, attempt, exception): now = time.time() diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_lb_public_ip.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_lb_public_ip.py index bf54ea684..b5de2c2e2 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_lb_public_ip.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_lb_public_ip.py @@ -12,7 +12,6 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -from kuryr.lib import exceptions as kl_exc import mock from neutronclient.common import exceptions as n_exc @@ -97,10 +96,10 @@ class TestFloatingIpServicePubIPDriverDriver(test_base.TestCase): spec_lb_ip, project_id)) @mock.patch('kuryr_kubernetes.config.CONF') - def test_acquire_service_pub_ip_info_pool_subnet_not_defined(self, m_cfg): + def test_acquire_service_pub_ip_info_pool_net_not_defined(self, m_cfg): driver = d_lb_public_ip.FloatingIpServicePubIPDriver() - public_subnet = '' - m_cfg.neutron_defaults.external_svc_subnet = public_subnet + public_net = '' + m_cfg.neutron_defaults.external_svc_net = public_net neutron = self.useFixture(k_fix.MockNeutronClient()).client neutron.list_floatingips.return_value = {} project_id = mock.sentinel.project_id @@ -113,22 +112,32 @@ class TestFloatingIpServicePubIPDriverDriver(test_base.TestCase): spec_type, spec_lb_ip, project_id) @mock.patch('kuryr_kubernetes.config.CONF') - def test_acquire_service_pub_ip_info_pool_subnet_not_exist(self, m_cfg): - driver = d_lb_public_ip.FloatingIpServicePubIPDriver() + def test_acquire_service_pub_ip_info_pool_subnet_is_none(self, m_cfg): + cls = d_lb_public_ip.FloatingIpServicePubIPDriver + m_driver = mock.Mock(spec=cls) + m_driver._drv_pub_ip = public_ip.FipPubIpDriver() neutron = self.useFixture(k_fix.MockNeutronClient()).client - public_subnet = mock.sentinel.public_subnet - m_cfg.neutron_defaults.external_svc_subnet = public_subnet + public_net = mock.sentinel.public_subnet + m_cfg.neutron_defaults.external_svc_net = public_net + m_cfg.neutron_defaults.external_svc_subnet = None - neutron.show_subnet.return_value = {} + neutron.show_subnet.return_value =\ + {'subnet': {'network_id': 'ec29d641-fec4-4f67-928a-124a76b3a8e6'}} + floating_ip = {'floating_ip_address': '1.2.3.5', + 'id': 'ec29d641-fec4-4f67-928a-124a76b3a888'} + neutron.create_floatingip.return_value = {'floatingip': floating_ip} project_id = mock.sentinel.project_id spec_type = 'LoadBalancer' spec_lb_ip = None - self.assertRaises( - kl_exc.NoResourceException, - driver.acquire_service_pub_ip_info, - spec_type, spec_lb_ip, project_id) + expected_resp = \ + obj_lbaas.LBaaSPubIp(ip_id=floating_ip['id'], + ip_addr=floating_ip['floating_ip_address'], + alloc_method='pool') + + self.assertEqual(expected_resp, cls.acquire_service_pub_ip_info + (m_driver, spec_type, spec_lb_ip, project_id)) @mock.patch('kuryr_kubernetes.config.CONF') def test_acquire_service_pub_ip_info_alloc_from_pool(self, m_cfg): diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_public_ip.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_public_ip.py index 77f2fc339..ffa7ba43f 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_public_ip.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_public_ip.py @@ -101,7 +101,7 @@ class TestFipPubIpDriver(test_base.TestCase): neutron.create_floatingip.return_value = {'floatingip': floating_ip} fip_id, fip_addr = cls.allocate_ip( - m_driver, pub_net_id, pub_subnet_id, project_id, description) + m_driver, pub_net_id, project_id, pub_subnet_id, description) self.assertEqual(fip_id, floating_ip['id']) self.assertEqual(fip_addr, floating_ip['floating_ip_address']) @@ -118,7 +118,7 @@ class TestFipPubIpDriver(test_base.TestCase): self.assertRaises( n_exc.NeutronClientException, cls.allocate_ip, - m_driver, pub_net_id, pub_subnet_id, project_id, description) + m_driver, pub_net_id, project_id, pub_subnet_id, description) def test_free_ip_neutron_exception(self): cls = d_public_ip.FipPubIpDriver diff --git a/kuryr_kubernetes/tests/unit/handlers/test_retry.py b/kuryr_kubernetes/tests/unit/handlers/test_retry.py index 849e4cacd..4e1be39d0 100644 --- a/kuryr_kubernetes/tests/unit/handlers/test_retry.py +++ b/kuryr_kubernetes/tests/unit/handlers/test_retry.py @@ -134,18 +134,3 @@ class TestRetryHandler(test_base.TestCase): m_sleep.assert_has_calls([ mock.call(deadline, i + 1, failures[i]) for i in range(len(failures))]) - - @mock.patch('itertools.count') - @mock.patch.object(h_retry.Retry, '_sleep') - def test_call_should_not_raise(self, m_sleep, m_count): - event = mock.sentinel.event - m_handler = mock.Mock() - m_handler.side_effect = _EX1() - m_count.return_value = list(range(1, 5)) - retry = h_retry.Retry(m_handler, exceptions=(_EX11, _EX2)) - - retry(event) - - m_handler.assert_called_with(event) - m_handler.set_health_status.assert_called_with(healthy=False) - m_sleep.assert_not_called() diff --git a/releasenotes/notes/make-ext-subnet-optional-99e73bfcbde96c22.yaml b/releasenotes/notes/make-ext-subnet-optional-99e73bfcbde96c22.yaml new file mode 100644 index 000000000..12dc0e9e5 --- /dev/null +++ b/releasenotes/notes/make-ext-subnet-optional-99e73bfcbde96c22.yaml @@ -0,0 +1,24 @@ +--- +upgrade: + - | + + For the external services (type=LoadBalancer) case, + a new field 'external_svc_net' was added and the 'external_svc_subnet' + field become optional. + The following should be modified at kuryr.conf:: + + + [neutron_defaults] + external_svc_net= + # 'external_svc_subnet' field is optional, set this field in case + # multiple subnets attached to 'external_svc_net' + external_svc_subnet= + +fixes: + - | + It is very common for production environments to only allow access + to the public network and not the associated public subnets. + In that case, we fail to allocate a floating IP to the Loadbalancer service + type. + In order to fix it, we added an option for specifying the network id instead + and switch the subnet config option to being optional. \ No newline at end of file