From 610f6e43cd540d8af508d564ead73100ce2a6e3f Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Mon, 5 Aug 2024 11:51:22 +0330 Subject: [PATCH] chore: applied the requested changes --- pkg/instance/build.go | 9 ++----- pkg/instance/errors.go | 1 + pkg/instance/execution.go | 14 ++++++++--- pkg/instance/instance.go | 2 +- pkg/instance/resources.go | 51 +++++++++++++++++++-------------------- pkg/instance/security.go | 22 ++++++++++++++++- pkg/knuu/instance_old.go | 2 +- pkg/knuu/knuu.go | 2 +- 8 files changed, 63 insertions(+), 40 deletions(-) diff --git a/pkg/instance/build.go b/pkg/instance/build.go index 8fd8f2e..6486b98 100644 --- a/pkg/instance/build.go +++ b/pkg/instance/build.go @@ -33,20 +33,15 @@ func (b *build) ImageName() string { } // SetImage sets the image of the instance. -// When calling in state 'Started', make sure to call AddVolume() before. -// It is only allowed in the 'None' and 'Started' states. +// It is only allowed in the 'None' and 'Preparing' states. func (b *build) SetImage(ctx context.Context, image string) error { - if !b.instance.IsInState(StateNone, StateStarted) { + if !b.instance.IsInState(StateNone, StatePreparing) { if b.instance.sidecars.IsSidecar() { return ErrSettingImageNotAllowedForSidecarsStarted } return ErrSettingImageNotAllowed.WithParams(b.instance.state.String()) } - if b.instance.state == StateStarted { - return b.setImageWithGracePeriod(ctx, image, nil) - } - // Use the builder to build a new image factory, err := container.NewBuilderFactory(image, b.getBuildDir(), b.instance.ImageBuilder) if err != nil { diff --git a/pkg/instance/errors.go b/pkg/instance/errors.go index 0c7daf9..45242db 100644 --- a/pkg/instance/errors.go +++ b/pkg/instance/errors.go @@ -215,4 +215,5 @@ var ( ErrInitializingSidecar = errors.New("InitializingSidecar", "error initializing sidecar for instance '%s'") ErrSidecarInstanceIsNil = errors.New("SidecarInstanceIsNil", "sidecar instance is nil for instance '%s'") ErrFailedToDeletePersistentVolumeClaim = errors.New("FailedToDeletePersistentVolumeClaim", "failed to delete persistent volume claim") + ErrUpgradingImageNotAllowed = errors.New("UpgradingImageNotAllowed", "upgrading image is only allowed in state 'Started'. Current state is '%s'") ) diff --git a/pkg/instance/execution.go b/pkg/instance/execution.go index c698580..c1659d4 100644 --- a/pkg/instance/execution.go +++ b/pkg/instance/execution.go @@ -266,6 +266,14 @@ func (e *execution) Destroy(ctx context.Context) error { return nil } +func (e *execution) UpgradeImage(ctx context.Context, image string) error { + if !e.instance.IsInState(StateStarted) { + return ErrUpgradingImageNotAllowed.WithParams(e.instance.state.String()) + } + + return e.instance.build.setImageWithGracePeriod(ctx, image, nil) +} + // BatchDestroy destroys a list of instances. func BatchDestroy(ctx context.Context, instances ...*Instance) error { if os.Getenv("KNUU_SKIP_CLEANUP") == "true" { @@ -313,8 +321,8 @@ func (e *execution) deployPod(ctx context.Context) error { } // create a role and role binding for the pod if there are policy rules - if len(e.instance.resources.policyRules) > 0 { - if err := e.instance.K8sClient.CreateRole(ctx, e.instance.k8sName, labels, e.instance.resources.policyRules); err != nil { + if len(e.instance.security.policyRules) > 0 { + if err := e.instance.K8sClient.CreateRole(ctx, e.instance.k8sName, labels, e.instance.security.policyRules); err != nil { return ErrFailedToCreateRole.Wrap(err) } if err := e.instance.K8sClient.CreateRoleBinding(ctx, e.instance.k8sName, labels, e.instance.k8sName, e.instance.k8sName); err != nil { @@ -352,7 +360,7 @@ func (e *execution) destroyPod(ctx context.Context) error { } // Delete the role and role binding for the pod if there are policy rules - if len(e.instance.resources.policyRules) == 0 { + if len(e.instance.security.policyRules) == 0 { return nil } diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go index bffbf73..8cbd83b 100644 --- a/pkg/instance/instance.go +++ b/pkg/instance/instance.go @@ -68,7 +68,6 @@ func New(name string, sysDeps system.SystemDependencies) (*Instance, error) { i.execution = &execution{instance: i} i.resources = &resources{ instance: i, - policyRules: make([]rbacv1.PolicyRule, 0), memoryRequest: resource.Quantity{}, memoryLimit: resource.Quantity{}, cpuRequest: resource.Quantity{}, @@ -95,6 +94,7 @@ func New(name string, sysDeps system.SystemDependencies) (*Instance, error) { instance: i, privileged: false, capabilitiesAdd: make([]string, 0), + policyRules: make([]rbacv1.PolicyRule, 0), } i.sidecars = &sidecars{ diff --git a/pkg/instance/resources.go b/pkg/instance/resources.go index 863cbe3..1c54aa4 100644 --- a/pkg/instance/resources.go +++ b/pkg/instance/resources.go @@ -3,14 +3,12 @@ package instance import ( "context" - rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime/schema" ) type resources struct { instance *Instance - policyRules []rbacv1.PolicyRule memoryRequest resource.Quantity memoryLimit resource.Quantity cpuRequest resource.Quantity @@ -20,16 +18,6 @@ func (i *Instance) Resources() *resources { return i.resources } -// AddPolicyRule adds a policy rule to the instance -// This function can only be called in the states 'Preparing' and 'Committed' -func (r *resources) AddPolicyRule(rule rbacv1.PolicyRule) error { - if !r.instance.IsInState(StatePreparing, StateCommitted) { - return ErrAddingPolicyRuleNotAllowed.WithParams(r.instance.state.String()) - } - r.policyRules = append(r.policyRules, rule) - return nil -} - // SetMemory sets the memory of the instance // This function can only be called in the states 'Preparing' and 'Committed' func (r *resources) SetMemory(request, limit resource.Quantity) error { @@ -76,18 +64,18 @@ func (r *resources) CustomResourceDefinitionExists(ctx context.Context, gvr *sch func (r *resources) deployResources(ctx context.Context) error { // only a non-sidecar instance should deploy a service, all sidecars will use the parent instance's service if !r.instance.sidecars.isSidecar { - portsTCP := r.instance.network.portsTCP - portsUDP := r.instance.network.portsUDP - for _, sidecar := range r.instance.sidecars.sidecars { - portsTCP = append(portsTCP, sidecar.Instance().network.portsTCP...) - portsUDP = append(portsUDP, sidecar.Instance().network.portsUDP...) - } - if len(portsTCP) != 0 || len(portsUDP) != 0 { - if err := r.instance.network.deployOrPatchService(ctx, portsTCP, portsUDP); err != nil { - return ErrFailedToDeployOrPatchService.Wrap(err) - } + if err := r.deployService(ctx); err != nil { + return err } } + + if err := r.deployStorage(ctx); err != nil { + return err + } + return nil +} + +func (r *resources) deployStorage(ctx context.Context) error { if len(r.instance.storage.volumes) != 0 { if err := r.instance.storage.deployVolume(ctx); err != nil { return ErrDeployingVolumeForInstance.WithParams(r.instance.k8sName).Wrap(err) @@ -103,6 +91,21 @@ func (r *resources) deployResources(ctx context.Context) error { return nil } +func (r *resources) deployService(ctx context.Context) error { + portsTCP := r.instance.network.portsTCP + portsUDP := r.instance.network.portsUDP + for _, sidecar := range r.instance.sidecars.sidecars { + portsTCP = append(portsTCP, sidecar.Instance().network.portsTCP...) + portsUDP = append(portsUDP, sidecar.Instance().network.portsUDP...) + } + if len(portsTCP) != 0 || len(portsUDP) != 0 { + if err := r.instance.network.deployOrPatchService(ctx, portsTCP, portsUDP); err != nil { + return ErrFailedToDeployOrPatchService.Wrap(err) + } + } + return nil +} + // destroyResources destroys the resources for the instance func (r *resources) destroyResources(ctx context.Context) error { if len(r.instance.storage.volumes) != 0 { @@ -140,16 +143,12 @@ func (r *resources) clone() *resources { return nil } - policyRulesCopy := make([]rbacv1.PolicyRule, len(r.policyRules)) - copy(policyRulesCopy, r.policyRules) - memoryRequestCopy := r.memoryRequest.DeepCopy() memoryLimitCopy := r.memoryLimit.DeepCopy() cpuRequestCopy := r.cpuRequest.DeepCopy() return &resources{ instance: nil, - policyRules: policyRulesCopy, memoryRequest: memoryRequestCopy, memoryLimit: memoryLimitCopy, cpuRequest: cpuRequestCopy, diff --git a/pkg/instance/security.go b/pkg/instance/security.go index 6ed8cb5..9c9238f 100644 --- a/pkg/instance/security.go +++ b/pkg/instance/security.go @@ -1,6 +1,9 @@ package instance -import v1 "k8s.io/api/core/v1" +import ( + v1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" +) // represents the security settings for a container type security struct { @@ -11,12 +14,25 @@ type security struct { // CapabilitiesAdd is the list of capabilities to add to the container capabilitiesAdd []string + + // PolicyRules is the list of policy rules to add to the instance + policyRules []rbacv1.PolicyRule } func (i *Instance) Security() *security { return i.security } +// AddPolicyRule adds a policy rule to the instance +// This function can only be called in the states 'Preparing' and 'Committed' +func (s *security) AddPolicyRule(rule rbacv1.PolicyRule) error { + if !s.instance.IsInState(StatePreparing, StateCommitted) { + return ErrAddingPolicyRuleNotAllowed.WithParams(s.instance.state.String()) + } + s.policyRules = append(s.policyRules, rule) + return nil +} + // SetPrivileged sets the privileged status for the instance // This function can only be called in the state 'Preparing' or 'Committed' func (s *security) SetPrivileged(privileged bool) error { @@ -79,9 +95,13 @@ func (s *security) clone() *security { capabilitiesAddCopy := make([]string, len(s.capabilitiesAdd)) copy(capabilitiesAddCopy, s.capabilitiesAdd) + policyRulesCopy := make([]rbacv1.PolicyRule, len(s.policyRules)) + copy(policyRulesCopy, s.policyRules) + return &security{ instance: nil, privileged: s.privileged, capabilitiesAdd: capabilitiesAddCopy, + policyRules: policyRulesCopy, } } diff --git a/pkg/knuu/instance_old.go b/pkg/knuu/instance_old.go index 571355b..f305549 100644 --- a/pkg/knuu/instance_old.go +++ b/pkg/knuu/instance_old.go @@ -177,7 +177,7 @@ func (i *Instance) ReadFileFromRunningInstance(ctx context.Context, filePath str // Deprecated: Use the new package knuu instead. func (i *Instance) AddPolicyRule(rule rbacv1.PolicyRule) error { - return i.Instance.Resources().AddPolicyRule(rule) + return i.Instance.Security().AddPolicyRule(rule) } // Deprecated: Use the new package knuu instead. diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index decddde..c1a19d0 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -141,7 +141,7 @@ func (k *Knuu) handleTimeout(ctx context.Context) error { Resources: []string{"*"}, } - if err := inst.Resources().AddPolicyRule(rule); err != nil { + if err := inst.Security().AddPolicyRule(rule); err != nil { return ErrCannotAddPolicyRule.Wrap(err) } if err := inst.Execution().Start(ctx); err != nil {