Skip to content

Commit

Permalink
Merge pull request #148 from booxter/non-root-northd
Browse files Browse the repository at this point in the history
northd: run as openvswitch:openvswitch
  • Loading branch information
openshift-ci[bot] authored Nov 1, 2023
2 parents c43bc69 + c274e27 commit 1135d0c
Show file tree
Hide file tree
Showing 13 changed files with 88 additions and 306 deletions.
5 changes: 0 additions & 5 deletions api/bases/ovn.openstack.org_ovnnorthds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,6 @@ spec:
- type
type: object
type: array
hash:
additionalProperties:
type: string
description: Map of hashes to track e.g. job status
type: object
networkAttachments:
additionalProperties:
items:
Expand Down
3 changes: 0 additions & 3 deletions api/v1beta1/ovnnorthd_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,6 @@ type OVNNorthdStatus struct {
// ReadyCount of OVN Northd instances
ReadyCount int32 `json:"readyCount,omitempty"`

// Map of hashes to track e.g. job status
Hash map[string]string `json:"hash,omitempty"`

// Conditions
Conditions condition.Conditions `json:"conditions,omitempty" optional:"true"`

Expand Down
7 changes: 0 additions & 7 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions config/crd/bases/ovn.openstack.org_ovnnorthds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,6 @@ spec:
- type
type: object
type: array
hash:
additionalProperties:
type: string
description: Map of hashes to track e.g. job status
type: object
networkAttachments:
additionalProperties:
items:
Expand Down
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,14 @@ rules:
- list
- update
- watch
- apiGroups:
- security.openshift.io
resourceNames:
- anyuid
resources:
- securitycontextconstraints
verbs:
- use
- apiGroups:
- security.openshift.io
resourceNames:
Expand Down
133 changes: 12 additions & 121 deletions controllers/ovnnorthd_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,10 @@ import (
"github.com/go-logr/logr"
"github.com/openstack-k8s-operators/lib-common/modules/common"
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
"github.com/openstack-k8s-operators/lib-common/modules/common/configmap"
"github.com/openstack-k8s-operators/lib-common/modules/common/deployment"
"github.com/openstack-k8s-operators/lib-common/modules/common/env"
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
"github.com/openstack-k8s-operators/lib-common/modules/common/labels"
nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment"
common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac"
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
"github.com/openstack-k8s-operators/ovn-operator/api/v1beta1"
ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1"
"github.com/openstack-k8s-operators/ovn-operator/pkg/ovnnorthd"
Expand Down Expand Up @@ -87,7 +83,7 @@ func (r *OVNNorthdReconciler) GetLogger(ctx context.Context) logr.Logger {
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=roles,verbs=get;list;watch;create;update
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=rolebindings,verbs=get;list;watch;create;update
// service account permissions that are needed to grant permission to the above
// +kubebuilder:rbac:groups="security.openshift.io",resourceNames=anyuid;privileged,resources=securitycontextconstraints,verbs=use
// +kubebuilder:rbac:groups="security.openshift.io",resourceNames=anyuid,resources=securitycontextconstraints,verbs=use
// +kubebuilder:rbac:groups="",resources=pods,verbs=create;delete;get;list;patch;update;watch

// Reconcile - OVN Northd
Expand Down Expand Up @@ -116,7 +112,6 @@ func (r *OVNNorthdReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
// initialize conditions used later as Status=Unknown
cl := condition.CreateList(
condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage),
condition.UnknownCondition(condition.ServiceConfigReadyCondition, condition.InitReason, condition.ServiceConfigReadyInitMessage),
condition.UnknownCondition(condition.NetworkAttachmentsReadyCondition, condition.InitReason, condition.NetworkAttachmentsReadyInitMessage),
condition.UnknownCondition(condition.DeploymentReadyCondition, condition.InitReason, condition.DeploymentReadyInitMessage),
condition.UnknownCondition(condition.ServiceAccountReadyCondition, condition.InitReason, condition.ServiceAccountReadyInitMessage),
Expand All @@ -131,9 +126,6 @@ func (r *OVNNorthdReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
return ctrl.Result{}, err
}
}
if instance.Status.Hash == nil {
instance.Status.Hash = map[string]string{}
}
if instance.Status.NetworkAttachments == nil {
instance.Status.NetworkAttachments = map[string][]string{}
}
Expand Down Expand Up @@ -214,9 +206,6 @@ func (r *OVNNorthdReconciler) reconcileUpdate(ctx context.Context, instance *ovn

Log.Info("Reconciling Service update")

// TODO: should have minor update tasks if required
// - delete dbsync hash from status to rerun it?

Log.Info("Reconciled Service update successfully")
return ctrl.Result{}, nil
}
Expand All @@ -226,9 +215,6 @@ func (r *OVNNorthdReconciler) reconcileUpgrade(ctx context.Context, instance *ov

Log.Info("Reconciling Service upgrade")

// TODO: should have major version upgrade tasks
// -delete dbsync hash from status to rerun it?

Log.Info("Reconciled Service upgrade successfully")
return ctrl.Result{}, nil
}
Expand All @@ -251,7 +237,7 @@ func (r *OVNNorthdReconciler) reconcileNormal(ctx context.Context, instance *ovn
rbacRules := []rbacv1.PolicyRule{
{
APIGroups: []string{"security.openshift.io"},
ResourceNames: []string{"anyuid", "privileged"},
ResourceNames: []string{"anyuid"},
Resources: []string{"securitycontextconstraints"},
Verbs: []string{"use"},
},
Expand All @@ -268,50 +254,8 @@ func (r *OVNNorthdReconciler) reconcileNormal(ctx context.Context, instance *ovn
return rbacResult, nil
}

// ConfigMap
configMapVars := make(map[string]env.Setter)

instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)

//
// Create ConfigMaps required as input for the Service and calculate an overall hash of hashes
//

//
// create Configmap required for northd input
// - %-scripts configmap holding scripts to e.g. bootstrap the service
// - %-config configmap holding minimal northd config required to get the service up, user can add additional files to be added to the service
// - parameters which has passwords gets added from the OpenStack secret via the init container
//
err = r.generateServiceConfigMaps(ctx, helper, instance, &configMapVars)
if err != nil {
instance.Status.Conditions.Set(condition.FalseCondition(
condition.ServiceConfigReadyCondition,
condition.ErrorReason,
condition.SeverityWarning,
condition.ServiceConfigReadyErrorMessage,
err.Error()))
return ctrl.Result{}, err
}

//
// create hash over all the different input resources to identify if any those changed
// and a restart/recreate is required.
//
inputHash, err := r.createHashOfInputHashes(ctx, instance, configMapVars)
if err != nil {
instance.Status.Conditions.Set(condition.FalseCondition(
condition.ServiceConfigReadyCondition,
condition.ErrorReason,
condition.SeverityWarning,
condition.ServiceConfigReadyErrorMessage,
err.Error()))
return ctrl.Result{}, err
}
// Create ConfigMaps and Secrets - end

instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage)

//
// TODO check when/if Init, Update, or Upgrade should/could be skipped
//
Expand Down Expand Up @@ -369,9 +313,18 @@ func (r *OVNNorthdReconciler) reconcileNormal(ctx context.Context, instance *ovn
return ctrlResult, nil
}

nbEndpoint, err := getInternalEndpoint(ctx, helper, instance, v1beta1.NBDBType)
if err != nil {
return ctrlResult, err
}
sbEndpoint, err := getInternalEndpoint(ctx, helper, instance, v1beta1.SBDBType)
if err != nil {
return ctrlResult, err
}

// Define a new Deployment object
depl := deployment.NewDeployment(
ovnnorthd.Deployment(instance, inputHash, serviceLabels, serviceAnnotations),
ovnnorthd.Deployment(instance, serviceLabels, serviceAnnotations, nbEndpoint, sbEndpoint),
time.Duration(5)*time.Second,
)

Expand Down Expand Up @@ -441,65 +394,3 @@ func getInternalEndpoint(
}
return internalEndpoint, nil
}

// generateServiceConfigMaps - create create configmaps which hold scripts and service configuration
// TODO add DefaultConfigOverwrite
func (r *OVNNorthdReconciler) generateServiceConfigMaps(
ctx context.Context,
h *helper.Helper,
instance *ovnv1.OVNNorthd,
envVars *map[string]env.Setter,
) error {
nbEndpoint, err := getInternalEndpoint(ctx, h, instance, v1beta1.NBDBType)
if err != nil {
return err
}
sbEndpoint, err := getInternalEndpoint(ctx, h, instance, v1beta1.SBDBType)
if err != nil {
return err
}

// Create/update configmaps from templates
templateParameters := make(map[string]interface{})
templateParameters["NBConnection"] = nbEndpoint
templateParameters["SBConnection"] = sbEndpoint
templateParameters["OVN_LOG_LEVEL"] = instance.Spec.LogLevel

cmLabels := labels.GetLabels(instance, labels.GetGroupLabel(ovnnorthd.ServiceName), map[string]string{})
cms := []util.Template{
// ConfigMap
{
Name: fmt.Sprintf("%s-config-data", instance.Name),
Namespace: instance.Namespace,
Type: util.TemplateTypeConfig,
InstanceType: instance.Kind,
Labels: cmLabels,
ConfigOptions: templateParameters,
},
}
return configmap.EnsureConfigMaps(ctx, h, instance, cms, envVars)
}

// createHashOfInputHashes - creates a hash of hashes which gets added to the resources which requires a restart
// if any of the input resources change, like configs, passwords, ...
func (r *OVNNorthdReconciler) createHashOfInputHashes(
ctx context.Context,
instance *ovnv1.OVNNorthd,
envVars map[string]env.Setter,
) (string, error) {
Log := r.GetLogger(ctx)

mergedMapVars := env.MergeEnvs([]corev1.EnvVar{}, envVars)
hash, err := util.ObjectHash(mergedMapVars)
if err != nil {
return hash, err
}
if hashMap, changed := util.SetHash(instance.Status.Hash, common.InputHashName, hash); changed {
instance.Status.Hash = hashMap
if err := r.Client.Status().Update(ctx, instance); err != nil {
return hash, err
}
Log.Info(fmt.Sprintf("Input maps hash %s - %s", common.InputHashName, hash))
}
return hash, nil
}
4 changes: 4 additions & 0 deletions pkg/ovnnorthd/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,8 @@ package ovnnorthd
const (
// ServiceName -
ServiceName = "ovn-northd"

// openvswitch:openvswitch
OVSUid int64 = 997
OVSGid int64 = 995
)
45 changes: 23 additions & 22 deletions pkg/ovnnorthd/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ limitations under the License.
package ovnnorthd

import (
"fmt"

"github.com/openstack-k8s-operators/lib-common/modules/common"
"github.com/openstack-k8s-operators/lib-common/modules/common/affinity"
"github.com/openstack-k8s-operators/lib-common/modules/common/env"
Expand All @@ -25,17 +27,17 @@ import (

const (
// ServiceCommand -
ServiceCommand = "/usr/local/bin/kolla_set_configs && /usr/local/bin/kolla_start"
ServiceCommand = "/usr/bin/ovn-northd"
)

// Deployment func
func Deployment(
instance *ovnv1.OVNNorthd,
configHash string,
labels map[string]string,
annotations map[string]string,
nbEndpoint string,
sbEndpoint string,
) *appsv1.Deployment {
runAsUser := int64(0)

livenessProbe := &corev1.Probe{
// TODO might need tuning
Expand All @@ -49,13 +51,21 @@ func Deployment(
PeriodSeconds: 5,
InitialDelaySeconds: 5,
}

noopCmd := []string{
"/bin/true",
cmd := ServiceCommand
args := []string{
"-vfile:off",
fmt.Sprintf("-vconsole:%s", instance.Spec.LogLevel),
fmt.Sprintf("--ovnnb-db=%s", nbEndpoint),
fmt.Sprintf("--ovnsb-db=%s", sbEndpoint),
}
args := []string{"-c"}

if instance.Spec.Debug.Service {
args = append(args, common.DebugCommand)
cmd = "/bin/sleep"
args = []string{"infinity"}

noopCmd := []string{
"/bin/true",
}
livenessProbe.Exec = &corev1.ExecAction{
Command: noopCmd,
}
Expand All @@ -64,7 +74,6 @@ func Deployment(
Command: noopCmd,
}
} else {
args = append(args, ServiceCommand)
//
// https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/
//
Expand All @@ -77,8 +86,6 @@ func Deployment(
}

envVars := map[string]env.Setter{}
envVars["KOLLA_CONFIG_STRATEGY"] = env.SetValue("COPY_ALWAYS")
envVars["CONFIG_HASH"] = env.SetValue(configHash)
// TODO: Make confs customizable
envVars["OVN_RUNDIR"] = env.SetValue("/tmp")

Expand All @@ -101,17 +108,12 @@ func Deployment(
ServiceAccountName: instance.RbacResourceName(),
Containers: []corev1.Container{
{
Name: ServiceName,
Command: []string{
"/bin/bash",
},
Args: args,
Image: instance.Spec.ContainerImage,
SecurityContext: &corev1.SecurityContext{
RunAsUser: &runAsUser,
},
Name: ServiceName,
Command: []string{cmd},
Args: args,
Image: instance.Spec.ContainerImage,
SecurityContext: getOVNNorthdSecurityContext(),
Env: env.MergeEnvs([]corev1.EnvVar{}, envVars),
VolumeMounts: GetNorthdVolumeMounts(),
Resources: instance.Spec.Resources,
ReadinessProbe: readinessProbe,
LivenessProbe: livenessProbe,
Expand All @@ -122,7 +124,6 @@ func Deployment(
},
},
}
deployment.Spec.Template.Spec.Volumes = GetNorthdVolumes(instance.Name)
// If possible two pods of the same service should not
// run on the same worker node. If this is not possible
// the get still created on the same worker node.
Expand Down
Loading

0 comments on commit 1135d0c

Please sign in to comment.