The lib-common from openstack-k8s-operators

setDatabaseHostname() panics if called in an environment where no MariaDB operator is deployed

Lines 63 to 91 in e6d7a13

    
           serviceList, err := common.GetServicesListWithLabel( 
        
           	ctx, 
        
           	h, 
        
           	h.GetBeforeObject().GetNamespace(), 
        
           	selector, 
        
           ) 
        
           if err != nil { 
        
           	msg := fmt.Sprintf("Error getting the DB service using label %v", selector) 
        
           	cond := condition.NewCondition( 
        
           		condition.TypeError, 
        
           		corev1.ConditionTrue, 
        
           		ReasonDBServiceNameError, 
        
           		msg) 
        
           	return cond, err 
        
           } 
        
           // can we expect there is only one DB instance per namespace? 
        
           if len(serviceList.Items) > 1 { 
        
           	msg := fmt.Sprintf("more then one DB service found %d", len(serviceList.Items)) 
        
           	cond := condition.NewCondition( 
        
           		condition.TypeError, 
        
           		corev1.ConditionTrue, 
        
           		ReasonDBServiceNameError, 
        
           		msg) 
        
           	return cond, err 
        
           } 
        
           d.databaseHostname = serviceList.Items[0].GetName()

GetServicesListWithLabel() does not return error if no Service is found, but returns an empty list, so serviceList.Items[0] leads to panic.

Enhancement: allow tuning tolerationSeconds default value

While looking into nodes failure detection and pods failover behavior/performance we stumbled upon:

https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/

The tolerationSeconds parameter allows you to specify how long a pod stays bound to a node that has a node condition. If the condition still exists after the tolerationSeconds period, the taint remains on the node and the pods with a matching toleration are evicted. If the condition clears before the tolerationSeconds period, pods with matching tolerations are not removed.

afaicu we don't explicitly set this tolerationSeconds value anywhere so it means each pod uses a default of 300s , resulting in workloads potentially taking more than five minutes to be rescheduled.

We are planning to document this and provide recommendations on how to accelerate the failover if required, still we thought it was worth bringing this up to see if we want to expose this parameter so that each operator can eventually set a more appropriate default value.

Action Required: Fix Renovate Configuration

There is an error with this repository's Renovate configuration that needs to be fixed. As a precaution, Renovate will stop PRs until it is resolved.

Error type: Cannot find preset's package (github>openstack-k8s-operators/renovate-config)

Enforce that every condition is initialized to Unknow state explicitly before use

As we documented in https://github.com/openstack-k8s-operators/docs/blob/main/conditions.md:

When a controller performs a task, before it starts a step condition gets initialized to Status=Unknown e.g. DBReady, DBSync, Endpoint, ServiceRegistered, …

However the current lib-common code:

lib-common/modules/common/condition/funcs.go

Lines 80 to 83 in 903adc9

    
           // If the condition does not exist, add it 
        
           if !exists { 
        
           	*conditions = append(*conditions, *c) 
        
           }

does not enforce this and allows creating new conditions in any state. We could change lib-common in a way that panics if a condition is created in other than Unknown state. This will force the service operators to follow the rules. The downside of this approach is that it creates runtime errors so it cannot be detect in build time.

CreateOrPatchDB fails if the CR Name and the requested Database schema name are different

Here the code uses h.GetBeforeObject().GetName() as the name of the new MariaDBDatabase CR

lib-common/modules/database/funcs.go

Lines 119 to 128 in 66e396f

    
           func (d *Database) CreateOrPatchDBWithLabel( 
        
           	ctx context.Context, 
        
           	h *helper.Helper, 
        
           	labels map[string]string, 
        
           ) (ctrl.Result, error) { 
        
           	db := &mariadbv1.MariaDBDatabase{ 
        
           		ObjectMeta: metav1.ObjectMeta{ 
        
           			Name:      h.GetBeforeObject().GetName(), 
        
           			Namespace: h.GetBeforeObject().GetNamespace(),

then later it uses the Database.databaseName as the name to look up the MariaDBDatabase CR

lib-common/modules/database/funcs.go

Lines 217 to 225 in 66e396f

    
           func (d *Database) getDBWithName( 
        
           	ctx context.Context, 
        
           	h *helper.Helper, 
        
           ) error { 
        
           	db := &mariadbv1.MariaDBDatabase{} 
        
           	err := h.GetClient().Get( 
        
           		ctx, 
        
           		types.NamespacedName{ 
        
           			Name:      d.databaseName,

So if the two name are not equal then the DB creation fails. I suggest to always used the database.databaseName.

Dependency Dashboard

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

Update module github.com/gophercloud/gophercloud to v2

Detected dependencies

gomod

modules/ansible/go.mod

gopkg.in/yaml.v3 v3.0.1

modules/certmanager/go.mod

github.com/cert-manager/cert-manager v1.11.5

github.com/go-logr/logr v1.4.2

github.com/google/uuid v1.6.0

github.com/onsi/ginkgo/v2 v2.19.0

github.com/onsi/gomega v1.33.1

go.uber.org/zap v1.27.0

k8s.io/api v0.28.12

k8s.io/apimachinery v0.28.12

k8s.io/client-go v0.28.12

k8s.io/utils v0.0.0-20240711033017-18e509b52bc8@18e509b52bc8

sigs.k8s.io/controller-runtime v0.16.6

github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0

k8s.io/apiextensions-apiserver v0.28.12

k8s.io/component-base v0.28.12

k8s.io/klog/v2 v2.110.1

modules/common/go.mod

github.com/go-logr/logr v1.4.2

github.com/google/uuid v1.6.0

github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0

github.com/onsi/ginkgo/v2 v2.19.0

github.com/onsi/gomega v1.33.1

github.com/pkg/errors v0.9.1

go.uber.org/zap v1.27.0

gopkg.in/yaml.v3 v3.0.1

k8s.io/api v0.28.12

k8s.io/apimachinery v0.28.12

k8s.io/client-go v0.28.12

k8s.io/utils v0.0.0-20240711033017-18e509b52bc8@18e509b52bc8

sigs.k8s.io/controller-runtime v0.16.6

k8s.io/apiextensions-apiserver v0.28.12

k8s.io/component-base v0.28.12

k8s.io/klog/v2 v2.110.1

modules/openstack/go.mod

github.com/go-logr/logr v1.4.2

github.com/gophercloud/gophercloud v1.13.0

k8s.io/api v0.28.12

k8s.io/apiextensions-apiserver v0.28.12

k8s.io/apimachinery v0.28.12

k8s.io/client-go v0.28.12

k8s.io/component-base v0.28.12

k8s.io/klog/v2 v2.110.1

k8s.io/utils v0.0.0-20240711033017-18e509b52bc8@18e509b52bc8

sigs.k8s.io/controller-runtime v0.16.6

modules/storage/go.mod

github.com/onsi/gomega v1.33.1

k8s.io/api v0.28.12

github.com/onsi/ginkgo/v2 v2.19.0

k8s.io/apimachinery v0.28.12

k8s.io/klog/v2 v2.110.1

Golang pre-commit hook has been sunsetted, we should pick another one

lib-common/.pre-commit-config.yaml

Line 2 in ea3a766

- repo: https://github.com/dnephin/pre-commit-golang

Modules are not in a shared workspace

Repo could use workspace to handle multiple modules. Workspaces were introduced in go 1.18 for use cases just like this one.[0]

[0]https://go.dev/blog/get-familiar-with-workspaces

openstack-k8s-operators / lib-common Goto Github PK

lib-common's People

Stargazers

Watchers

Forkers

lib-common's Issues

setDatabaseHostname() panics if called in an environment where no MariaDB operator is deployed

Enhancement: allow tuning tolerationSeconds default value

Action Required: Fix Renovate Configuration

Enforce that every condition is initialized to Unknow state explicitly before use

CreateOrPatchDB fails if the CR Name and the requested Database schema name are different

Dependency Dashboard

Open

Detected dependencies

Golang pre-commit hook has been sunsetted, we should pick another one

Modules are not in a shared workspace

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	serviceList, err := common.GetServicesListWithLabel(
	ctx,
	h,
	h.GetBeforeObject().GetNamespace(),
	selector,
	)
	if err != nil {
	msg := fmt.Sprintf("Error getting the DB service using label %v", selector)
	cond := condition.NewCondition(
	condition.TypeError,
	corev1.ConditionTrue,
	ReasonDBServiceNameError,
	msg)

	return cond, err
	}

	// can we expect there is only one DB instance per namespace?
	if len(serviceList.Items) > 1 {
	msg := fmt.Sprintf("more then one DB service found %d", len(serviceList.Items))
	cond := condition.NewCondition(
	condition.TypeError,
	corev1.ConditionTrue,
	ReasonDBServiceNameError,
	msg)

	return cond, err
	}
	d.databaseHostname = serviceList.Items[0].GetName()

	// If the condition does not exist, add it
	if !exists {
	conditions = append(conditions, *c)
	}

	func (d *Database) CreateOrPatchDBWithLabel(
	ctx context.Context,
	h *helper.Helper,
	labels map[string]string,
	) (ctrl.Result, error) {

	db := &mariadbv1.MariaDBDatabase{
	ObjectMeta: metav1.ObjectMeta{
	Name: h.GetBeforeObject().GetName(),
	Namespace: h.GetBeforeObject().GetNamespace(),

	func (d *Database) getDBWithName(
	ctx context.Context,
	h *helper.Helper,
	) error {
	db := &mariadbv1.MariaDBDatabase{}
	err := h.GetClient().Get(
	ctx,
	types.NamespacedName{
	Name: d.databaseName,