Skip to content

Commit

Permalink
Broker class based defaults (#7631)
Browse files Browse the repository at this point in the history 
* First draft of the default broker class code

Signed-off-by: Leo Li <leoli@redhat.com>

* Change the default broker class config

Signed-off-by: Leo Li <leoli@redhat.com>

* Run codegen

Signed-off-by: Leo Li <leoli@redhat.com>

* Refactor the code

Signed-off-by: Leo Li <leoli@redhat.com>

* Codegen

Signed-off-by: Leo Li <leoli@redhat.com>

* Fix the build error and run code gen

Signed-off-by: Leo Li <leoli@redhat.com>

* Fix the build error in the tests

Signed-off-by: Leo Li <leoli@redhat.com>

* Fix the build error in the tests

Signed-off-by: Leo Li <leoli@redhat.com>

* Modify the current existing test, and add more test cases

Signed-off-by: Leo Li <leoli@redhat.com>

* Modify the current existing test, and add more test cases

Signed-off-by: Leo Li <leoli@redhat.com>

* Pass in the actual broker name instead of leaving it empty

Signed-off-by: Leo Li <leoli@redhat.com>

* Modify the condition to check whether Different Namespace is allowed according to the default configs

Signed-off-by: Leo Li <leoli@redhat.com>

* Revert the brokerSpec.Name change.

Signed-off-by: Leo Li <leoli@redhat.com>

* Fix the failing unit tests by adding the default config for the defaults

Signed-off-by: Leo Li <leoli@redhat.com>

* Fix the failing unit tests

Signed-off-by: Leo Li <leoli@redhat.com>

* Fix the review comments

Signed-off-by: Leo Li <leoli@redhat.com>

* Update pkg/apis/config/defaults.go

Co-authored-by: Christoph Stäbler <cstabler@redhat.com>

* Change the tests to be table form

Signed-off-by: Leo Li <leoli@redhat.com>

* Check if the brokerClassName is the default broker class for the whole cluster

Signed-off-by: Leo Li <leoli@redhat.com>

* Fixed the build error

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: instead of always passing the empty className, pass in the variable instead

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: brokerClass is a string pointer. If the value is null but pointer has value, it will pass. So this commit fix that problem.

Signed-off-by: Leo Li <leoli@redhat.com>

* feat: adding some temporary test cases for testing purposes

Signed-off-by: Leo Li <leoli@redhat.com>

* feat: adding some temporary test cases for testing purposes

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: fix the failing tests

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: fix FIXME, the issue that can potentially cause SegFault

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: fix the failing rabbitmq issue

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: remove the trailing whitespace

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: update the comments

Signed-off-by: Leo Li <leoli@redhat.com>

* feat: add the high level summary for the code

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: fix the review comments

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: fix the review comments

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: fix Calum's review comments

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: fix failed unit test caused by the latest review comment fix

Signed-off-by: Leo Li <leoli@redhat.com>

* fix: fix failed unit test caused by the latest review comment fix

Signed-off-by: Leo Li <leoli@redhat.com>

* feat: add the test for the deprecating default broker config feature

Signed-off-by: Leo Li <leoli@redhat.com>

---------

Signed-off-by: Leo Li <leoli@redhat.com>
Co-authored-by: Christoph Stäbler <cstabler@redhat.com>
  • Loading branch information
@Leo6Leo @creydr
Leo6Leo and creydr committed Aug 22, 2024
1 parent e4b6d68 commit 36e0721
Show file tree
Hide file tree
Showing 10 changed files with 1,279 additions and 201 deletions.
199 changes: 169 additions & 30 deletions pkg/apis/config/defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,65 +68,204 @@ func NewDefaultsConfigFromConfigMap(config *corev1.ConfigMap) (*Defaults, error)
return NewDefaultsConfigFromMap(config.Data)
}

/*
The priority precedence for determining the broker class and configuration is as follows:
1. If a specific broker class is provided:
a. Check namespace-specific configuration for the given broker class
b. If not found, check cluster-wide configuration for the given broker class
c. If still not found, use the cluster-wide default configuration
2. If no specific broker class is provided:
a. Check namespace-specific default broker class
b. If found, use its configuration (following step 1)
c. If not found, use cluster-wide default broker class and configuration
3. If no default cluster configuration is set, return an error
4. If no default namespace configuration is set, will use the cluster-wide default configuration.
This can be represented as a flow chart:
Start
|
v
Is broker class provided?
/ \
Yes No
/ \
Check namespace config Check namespace
for provided class default class
| |
v v
Found? Found?
/ \ / \
Yes No Yes No
| | | |
| | | |
| v | v
| Check cluster | Use cluster
| config for class | default class
| | | and config
| v |
| Found? |
| / \ |
| Yes No |
| | | |
| | v |
| | Use cluster |
| | default config |
v v v
Use found configuration
The system prioritizes namespace-specific configurations over cluster-wide defaults,
and explicitly provided broker classes over default classes.
*/

// Defaults includes the default values to be populated by the webhook.
type Defaults struct {
// NamespaceDefaultsConfig are the default Broker Configs for each namespace.
// Namespace is the key, the value is the KReference to the config.
NamespaceDefaultsConfig map[string]*ClassAndBrokerConfig `json:"namespaceDefaults,omitempty"`
// Namespace is the key, the value is the NamespaceDefaultConfig
NamespaceDefaultsConfig map[string]*DefaultConfig `json:"namespaceDefaults,omitempty"`

// ClusterDefaultBrokerConfig is the default broker config for all the namespaces that
// are not in NamespaceDefaultBrokerConfigs.
ClusterDefault *ClassAndBrokerConfig `json:"clusterDefault,omitempty"`
// ClusterDefaultConfig is the default broker config for all the namespaces that
// are not in NamespaceDefaultBrokerConfigs. Different broker class could have
// different default config.
ClusterDefaultConfig *DefaultConfig `json:"clusterDefault,omitempty"`
}

// ClassAndBrokerConfig contains configuration for a given namespace for broker. Allows
// configuring the Class of the Broker, the reference to the
// config it should use and it's delivery.
type ClassAndBrokerConfig struct {
BrokerClass string `json:"brokerClass,omitempty"`
// DefaultConfig struct contains the default configuration for the cluster and namespace.
type DefaultConfig struct {
// DefaultBrokerClass and DefaultBrokerClassConfig are the default broker class for the whole cluster/namespace.
// Users have to specify both of them
DefaultBrokerClass string `json:"brokerClass,omitempty"`

//Deprecated: Use the config in BrokerClasses instead
*BrokerConfig `json:",inline"`

// Optional: BrokerClasses are the default broker classes' config. The key is the broker class name, and the value is the config for that broker class.
BrokerClasses map[string]*BrokerConfig `json:"brokerClasses,omitempty"`

DisallowDifferentNamespaceConfig *bool `json:"disallowDifferentNamespaceConfig,omitempty"`
}

// BrokerConfig contains configuration for a given namespace for broker. Allows
// BrokerConfig contains configuration for a given broker. Allows
// configuring the reference to the
// config it should use and it's delivery.
type BrokerConfig struct {
*duckv1.KReference `json:",inline"`
Delivery *eventingduckv1.DeliverySpec `json:"delivery,omitempty"`
}

// GetBrokerConfig returns a namespace specific Broker Configuration, and if
// that doesn't exist, return a Cluster Default and if that doesn't exist
// return an error.
func (d *Defaults) GetBrokerConfig(ns string) (*BrokerConfig, error) {
// GetBrokerConfig returns a namespace specific Broker Config, and if
// that doesn't exist, return a Cluster Default and if that doesn't exist return an error.
func (d *Defaults) GetBrokerConfig(ns string, brokerClassName *string) (*BrokerConfig, error) {
if d == nil {
return nil, errors.New("Defaults are nil")
return nil, errors.New("Defaults for Broker Configurations for cluster have not been set up. You can set them via ConfigMap config-br-defaults.")
}

// Early return if brokerClassName is provided and valid
if brokerClassName != nil && *brokerClassName != "" {
return d.getBrokerConfigByClassName(ns, *brokerClassName)
}

// Handling empty brokerClassName
return d.getBrokerConfigForEmptyClassName(ns)
}

// getBrokerConfigByClassName returns the BrokerConfig for the given brokerClassName.
// It first checks the namespace specific configuration, then the cluster default configuration.
func (d *Defaults) getBrokerConfigByClassName(ns string, brokerClassName string) (*BrokerConfig, error) {
// Check namespace specific configuration
if nsConfig, ok := d.NamespaceDefaultsConfig[ns]; ok && nsConfig != nil {
// check if the brokerClassName is the default broker class for this namespace
if nsConfig.DefaultBrokerClass == brokerClassName {
if nsConfig.BrokerConfig == nil {
// as no default broker class config is set for this namespace, check whether nsconfig's brokerClasses map has the config for this broker class
if config, ok := nsConfig.BrokerClasses[brokerClassName]; ok && config != nil {
return config, nil
}
// if not found, return the cluster default config
return d.getClusterDefaultBrokerConfig(brokerClassName)
} else {
// If the brokerClassName exists in the BrokerClasses, return the config in the BrokerClasses
if config, ok := nsConfig.BrokerClasses[brokerClassName]; ok && config != nil {
return config, nil
} else {
// If the brokerClassName is the default broker class for the namespace, return the BrokerConfig
return nsConfig.BrokerConfig, nil
}
}
} else {
// if the brokerClassName is not the default broker class for the namespace, check whether nsconfig's brokerClasses map has the config for this broker class
if config, ok := nsConfig.BrokerClasses[brokerClassName]; ok && config != nil {
return config, nil
}
}
}
value, present := d.NamespaceDefaultsConfig[ns]
if present && value.BrokerConfig != nil {
return value.BrokerConfig, nil

// Check cluster default configuration
return d.getClusterDefaultBrokerConfig(brokerClassName)
}

// getBrokerConfigForEmptyClassName returns the BrokerConfig for the given namespace when brokerClassName is empty.
// It first checks the namespace specific configuration, then the cluster default configuration.
func (d *Defaults) getBrokerConfigForEmptyClassName(ns string) (*BrokerConfig, error) {
// Check if namespace has a default broker class
if nsConfig, ok := d.NamespaceDefaultsConfig[ns]; ok && nsConfig != nil {
if nsConfig.DefaultBrokerClass != "" {
return d.getBrokerConfigByClassName(ns, nsConfig.DefaultBrokerClass)
}
}
if d.ClusterDefault != nil && d.ClusterDefault.BrokerConfig != nil {
return d.ClusterDefault.BrokerConfig, nil

// Fallback to cluster default configuration
return d.getClusterDefaultBrokerConfig("")
}

// getClusterDefaultBrokerConfig returns the BrokerConfig for the given brokerClassName.
func (d *Defaults) getClusterDefaultBrokerConfig(brokerClassName string) (*BrokerConfig, error) {
if d.ClusterDefaultConfig == nil || d.ClusterDefaultConfig.BrokerConfig == nil {
return nil, errors.New("Defaults for Broker Configurations for cluster have not been set up. You can set them via ConfigMap config-br-defaults.")
}
return nil, errors.New("Defaults for Broker Configurations have not been set up.")

// Check if the brokerClassName is the default broker class for the whole cluster
if brokerClassName == "" || d.ClusterDefaultConfig.DefaultBrokerClass == brokerClassName {
// If the brokerClassName exists in the BrokerClasses, return the config in the BrokerClasses
if config, ok := d.ClusterDefaultConfig.BrokerClasses[brokerClassName]; ok && config != nil {
return config, nil
} else {
// If the brokerClassName is the default broker class for the cluster, return the BrokerConfig
return d.ClusterDefaultConfig.BrokerConfig, nil
}
}

if config, ok := d.ClusterDefaultConfig.BrokerClasses[brokerClassName]; ok && config != nil {
return config, nil
}

return d.ClusterDefaultConfig.BrokerConfig, nil
}

// GetBrokerClass returns a namespace specific Broker Class, and if
// that doesn't exist, return a Cluster Default and if that doesn't exist
// that doesn't exist, return a Cluster Default and if the defaults doesn't exist
// return an error.
func (d *Defaults) GetBrokerClass(ns string) (string, error) {
if d == nil {
return "", errors.New("Defaults are nil")
return "", errors.New("Defaults for Broker Configurations for cluster have not been set up. You can set them via ConfigMap config-br-defaults.")
}
value, present := d.NamespaceDefaultsConfig[ns]
if present && value.BrokerClass != "" {
return value.BrokerClass, nil

// Check if the namespace has a specific configuration
if nsConfig, ok := d.NamespaceDefaultsConfig[ns]; ok && nsConfig != nil {
if nsConfig.DefaultBrokerClass != "" {
return nsConfig.DefaultBrokerClass, nil
}
}
if d.ClusterDefault != nil && d.ClusterDefault.BrokerClass != "" {
return d.ClusterDefault.BrokerClass, nil

// Fallback to cluster default configuration if namespace specific configuration is not set
if d.ClusterDefaultConfig != nil && d.ClusterDefaultConfig.DefaultBrokerClass != "" {
return d.ClusterDefaultConfig.DefaultBrokerClass, nil
}
return "", errors.New("Defaults for Broker Configurations have not been set up.")

// If neither namespace specific nor cluster default broker class is found
return "", fmt.Errorf("Neither namespace specific nor cluster default broker class is found for namespace %q, please set them via ConfigMap config-br-defaults", ns)
}
Loading

0 comments on commit 36e0721

Please sign in to comment.