Skip to content

Commit

Permalink
Implement per-route options in Cloud Controller (#4080)
Browse files Browse the repository at this point in the history
* Introduce per-route options

This commit adds a configurable load-balancing algorithm as a first example of per-route options.
It adds the 'options' field to the route object in the V3 API, and the app manifest.
The options field is an object storing key-value pairs, with 'lb_algo' being the only supported key for now.
The supported load-balancing algorithms are 'round-robin' and 'least-connections'.
The options field is introduced as a column in the database route table, and forwarded to the Diego backend.

Co-authored-by: Alexander Nicke <[email protected]>
See: cloudfoundry/capi-release#482
See: https://github.com/cloudfoundry/community/blob/main/toc/rfc/rfc-0027-generic-per-route-features.md

* Add route-options documentation

Add documentation for the route options object, and its supported fields.

* Adjust route options behaviour for manifest push

Overwrite behaviour for route options is now fixed and tested:
Existing options are not modified if options is nil, {} or not provided
A single option (e.g. loadbalancing-algorithm) can be removed by setting its value to nil

adjust test for manifest push:
options {key:nil} should not modify the existing value

* Adjust behaviour to new decisions:

options default: {}
API:
 options is not nullable
 specific option is additive
 specific option is nullable
 empty hash does not change anything (additive)
 get empty options -> {}
manifest:
 options and specific option is nullable, but no-op

* Remove route option validations from manifest_route

* Rename 'lb_algo' and 'loadbalancing-algorithm' to 'algorithm'

* Disallow null in manifest; Cleanup error outputs

---------

Co-authored-by: Alexander Nicke <[email protected]>
  • Loading branch information
hoffmaen and a18e authored Dec 10, 2024
1 parent 36ba7ed commit 05e617f
Show file tree
Hide file tree
Showing 37 changed files with 1,028 additions and 96 deletions.
11 changes: 10 additions & 1 deletion app/actions/manifest_route_update.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'cloud_controller/app_manifest/manifest_route'
require 'actions/route_create'
require 'actions/route_update'

module VCAP::CloudController
class ManifestRouteUpdate
Expand Down Expand Up @@ -81,7 +82,8 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info)
message = RouteCreateMessage.new({
'host' => host,
'path' => manifest_route[:path],
'port' => manifest_route[:port]
'port' => manifest_route[:port],
'options' => manifest_route[:options]
})

route = RouteCreate.new(user_audit_info).create(
Expand All @@ -92,6 +94,13 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info)
)
elsif route.space.guid != app.space_guid
raise InvalidRoute.new('Routes cannot be mapped to destinations in different spaces')
elsif manifest_route[:options] && route[:options] != manifest_route[:options]
# remove nil values from options
manifest_route[:options] = manifest_route[:options].compact
message = RouteUpdateMessage.new({
'options' => manifest_route[:options]
})
route = RouteUpdate.new.update(route:, message:)
end

return route
Expand Down
3 changes: 2 additions & 1 deletion app/actions/route_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ def create(message:, space:, domain:, manifest_triggered: false)
path: message.path || '',
port: port(message, domain),
space: space,
domain: domain
domain: domain,
options: message.options.nil? ? {} : message.options.compact
)

Route.db.transaction do
Expand Down
3 changes: 3 additions & 0 deletions app/actions/route_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ module VCAP::CloudController
class RouteUpdate
def update(route:, message:)
Route.db.transaction do
route.options = route.options.symbolize_keys.merge(message.options).compact if message.requested?(:options)

route.save
MetadataUpdate.update(route, message)
end

Expand Down
49 changes: 47 additions & 2 deletions app/messages/manifest_routes_update_message.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'messages/base_message'
require 'messages/route_options_message'
require 'cloud_controller/app_manifest/manifest_route'

module VCAP::CloudController
Expand All @@ -25,22 +26,66 @@ def contains_non_route_hash_values?(routes)
validates_with ManifestRoutesYAMLValidator, if: proc { |record| record.requested?(:routes) }
validate :routes_are_uris, if: proc { |record| record.requested?(:routes) }
validate :route_protocols_are_valid, if: proc { |record| record.requested?(:routes) }
validate :route_options_are_valid, if: proc { |record| record.requested?(:routes) }
validate :loadbalancings_are_valid, if: proc { |record| record.requested?(:routes) }
validate :no_route_is_boolean
validate :default_route_is_boolean
validate :random_route_is_boolean
validate :random_route_and_default_route_conflict

def manifest_route_mappings
@manifest_route_mappings ||= routes.map do |route|
{
route: ManifestRoute.parse(route[:route]),
r = {
route: ManifestRoute.parse(route[:route], route[:options]),
protocol: route[:protocol]
}
r[:options] = route[:options] unless route[:options].nil?
r
end
end

private

def route_options_are_valid
return if errors[:routes].present?

routes.any? do |r|
next unless r.keys.include?(:options)

unless r[:options].is_a?(Hash)
errors.add(:base, message: "Route '#{r[:route]}': options must be an object")
next
end

r[:options].each_key do |key|
RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) &&
errors.add(:base,
message: "Route '#{r[:route]}' contains invalid route option '#{key}'. \
Valid keys: '#{RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.join(', ')}'")
end
end
end

def loadbalancings_are_valid
return if errors[:routes].present?

routes.each do |r|
next unless r.keys.include?(:options) && r[:options].is_a?(Hash) && r[:options].keys.include?(:loadbalancing)

loadbalancing = r[:options][:loadbalancing]
unless loadbalancing.is_a?(String)
errors.add(:base,
message: "Invalid value for 'loadbalancing' for Route '#{r[:route]}'; \
Valid values are: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'")
next
end
RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(loadbalancing) &&
errors.add(:base,
message: "Cannot use loadbalancing value '#{loadbalancing}' for Route '#{r[:route]}'; \
Valid values are: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'")
end
end

def routes_are_uris
return if errors[:routes].present?

Expand Down
11 changes: 11 additions & 0 deletions app/messages/route_create_message.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'messages/metadata_base_message'
require 'messages/route_options_message'

module VCAP::CloudController
class RouteCreateMessage < MetadataBaseMessage
Expand All @@ -10,8 +11,13 @@ class RouteCreateMessage < MetadataBaseMessage
path
port
relationships
options
]

def self.options_requested?
@options_requested ||= proc { |a| a.requested?(:options) }
end

validates :host,
allow_nil: true,
string: true,
Expand Down Expand Up @@ -56,6 +62,7 @@ class RouteCreateMessage < MetadataBaseMessage

validates_with NoAdditionalKeysValidator
validates_with RelationshipValidator
validates_with OptionsValidator, if: options_requested?

delegate :space_guid, to: :relationships_message
delegate :domain_guid, to: :relationships_message
Expand All @@ -65,6 +72,10 @@ def relationships_message
@relationships_message ||= Relationships.new(relationships&.deep_symbolize_keys)
end

def options_message
@options_message ||= RouteOptionsMessage.new(options&.deep_symbolize_keys)
end

def wildcard?
host == '*'
end
Expand Down
16 changes: 16 additions & 0 deletions app/messages/route_options_message.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
require 'messages/metadata_base_message'

module VCAP::CloudController
class RouteOptionsMessage < BaseMessage
VALID_MANIFEST_ROUTE_OPTIONS = %i[loadbalancing].freeze
VALID_ROUTE_OPTIONS = %i[loadbalancing].freeze
VALID_LOADBALANCING_ALGORITHMS = %w[round-robin least-connections].freeze

register_allowed_keys VALID_ROUTE_OPTIONS
validates_with NoAdditionalKeysValidator
validates :loadbalancing,
inclusion: { in: VALID_LOADBALANCING_ALGORITHMS, message: "must be one of '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}' if present" },
presence: true,
allow_nil: true
end
end
13 changes: 12 additions & 1 deletion app/messages/route_update_message.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
require 'messages/metadata_base_message'
require 'messages/route_options_message'

module VCAP::CloudController
class RouteUpdateMessage < MetadataBaseMessage
register_allowed_keys []
register_allowed_keys %i[options]

def self.options_requested?
@options_requested ||= proc { |a| a.requested?(:options) }
end

def options_message
@options_message ||= RouteOptionsMessage.new(options&.deep_symbolize_keys)
end

validates_with OptionsValidator, if: options_requested?

validates_with NoAdditionalKeysValidator
end
Expand Down
17 changes: 17 additions & 0 deletions app/messages/validators.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,23 @@ def validate(record)
end
end

class OptionsValidator < ActiveModel::Validator
def validate(record)
unless record.options.is_a?(Hash)
record.errors.add(:options, message: "'options' is not a valid object")
return
end

opt = record.options_message

return if opt.valid?

opt.errors.full_messages.each do |message|
record.errors.add(:options, message:)
end
end
end

class ToOneRelationshipValidator < ActiveModel::EachValidator
def validate_each(record, attribute, relationship)
if has_correct_structure?(relationship)
Expand Down
21 changes: 19 additions & 2 deletions app/models/runtime/route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ class OutOfVIPException < CloudController::Errors::InvalidRelation; end

add_association_dependencies route_mappings: :destroy

export_attributes :host, :path, :domain_guid, :space_guid, :service_instance_guid, :port
import_attributes :host, :path, :domain_guid, :space_guid, :app_guids, :port
export_attributes :host, :path, :domain_guid, :space_guid, :service_instance_guid, :port, :options
import_attributes :host, :path, :domain_guid, :space_guid, :app_guids, :port, :options

add_association_dependencies labels: :destroy
add_association_dependencies annotations: :destroy
Expand Down Expand Up @@ -71,6 +71,23 @@ def as_summary_json
}
end

def options_with_serialization=(opts)
self.options_without_serialization = Oj.dump(opts)
end

alias_method :options_without_serialization=, :options=
alias_method :options=, :options_with_serialization=

def options_with_serialization
string = options_without_serialization
return nil if string.blank?

Oj.load(string)
end

alias_method :options_without_serialization, :options
alias_method :options, :options_with_serialization

alias_method :old_path, :path
def path
old_path.nil? ? '' : old_path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,17 @@ module AppManifestPresenters
class RoutePropertiesPresenter
def to_hash(route_mappings:, app:, **_)
route_hashes = route_mappings.map do |route_mapping|
{
route_hash = {
route: route_mapping.route.uri,
protocol: route_mapping.protocol
}

if route_mapping.route.options
route_hash[:options] = {}
route_hash[:options][:loadbalancing] = route_mapping.route.options[:loadbalancing] if route_mapping.route.options[:loadbalancing]
end

route_hash
end

{ routes: alphabetize(route_hashes).presence }
Expand Down
1 change: 1 addition & 0 deletions app/presenters/v3/route_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def to_hash
},
links: build_links
}
hash.merge!(options: route.options) unless route.options.nil?

@decorators.reduce(hash) { |memo, d| d.decorate(memo, [route]) }
end
Expand Down
12 changes: 12 additions & 0 deletions db/migrations/20241105000000_add_options_to_route.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Sequel.migration do
up do
alter_table(:routes) do
add_column :options, String, size: 4096, default: '{}'
end
end
down do
alter_table(:routes) do
drop_column :options
end
end
end
12 changes: 12 additions & 0 deletions docs/v3/source/includes/api_resources/_routes.erb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
"protocol": "tcp"
}
],
"options": {
"loadbalancing": "round-robin"
},
"metadata": {
"labels": <%= metadata.fetch(:labels, {}).to_json(space: ' ', object_nl: ' ')%>,
"annotations": <%= metadata.fetch(:annotations, {}).to_json(space: ' ', object_nl: ' ')%>
Expand Down Expand Up @@ -124,6 +127,9 @@
"protocol": "http1"
}
],
"options": {
"loadbalancing": "round-robin"
},
"metadata": {
"labels": {},
"annotations": {}
Expand Down Expand Up @@ -212,3 +218,9 @@
"protocol": "http2"
}
<% end %>

<% content_for :route_options do %>
{
"loadbalancing": "round-robin"
}
<% end %>
26 changes: 15 additions & 11 deletions docs/v3/source/includes/resources/routes/_create.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ curl "https://api.example.org/v3/routes" \
"data": { "guid": "space-guid" }
}
},
"options": {
"loadbalancing": "round-robin"
}
"metadata": {
"labels": { "key": "value" },
"annotations": { "note": "detailed information"}
Expand All @@ -44,20 +47,21 @@ Content-Type: application/json

#### Required parameters

| Name | Type | Description |
| ----------------------------------------- | ---------------------------------------------- | ---------------------------------------------------------------------------------------------------------- |
| **relationships.space** | [_to-one relationship_](#to-one-relationships) | A relationship to the space containing the route; routes can only be mapped to destinations in that space |
| **relationships.domain** | [_to-one relationship_](#to-one-relationships) | A relationship to the domain of the route |
| Name | Type | Description |
|--------------------------|------------------------------------------------|-----------------------------------------------------------------------------------------------------------|
| **relationships.space** | [_to-one relationship_](#to-one-relationships) | A relationship to the space containing the route; routes can only be mapped to destinations in that space |
| **relationships.domain** | [_to-one relationship_](#to-one-relationships) | A relationship to the domain of the route |

#### Optional parameters

| Name | Type | Description |
| ------------------------ | ----------------------------------- | --------------------------------- |
| **host** | _string_ | The host component for the route; not compatible with routes specifying the `tcp` protocol |
| **path** | _string_ | The path component for the route; should begin with a `/` and not compatible with routes specifying the `tcp` protocol |
| **port** | _integer_ | The port the route will listen on; only compatible with routes leveraging a domain that supports the `tcp` protocol. For `tcp` domains, a port will be randomly assigned if not specified
| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route |
| **metadata.labels** | [_label object_](#labels) | Labels applied to the route |
| Name | Type | Description |
|--------------------------|----------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| **host** | _string_ | The host component for the route; not compatible with routes specifying the `tcp` protocol |
| **path** | _string_ | The path component for the route; should begin with a `/` and not compatible with routes specifying the `tcp` protocol |
| **port** | _integer_ | The port the route will listen on; only compatible with routes leveraging a domain that supports the `tcp` protocol. For `tcp` domains, a port will be randomly assigned if not specified |
| **options** | [_route option object_](#the-route-options-object) | Options applied to the route |
| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the route |
| **metadata.labels** | [_label object_](#labels) | Labels applied to the route |

#### Permitted roles

Expand Down
Loading

0 comments on commit 05e617f

Please sign in to comment.