Skip to content

Commit

Permalink
Handle upgrade from older version which has pruner defaults
Browse files Browse the repository at this point in the history
Before webhook we had default value for pruner's keep as 1
but we expect user to define all values now so if a user has
installed prev version and has not enabled pruner then `keep`
will have a value 1 and after upgrading to newer version,
webhook will fail if keep has a value and other fields are not defined
this handles that case by removing the default for keep if
other pruner fields are not defined

Signed-off-by: Shivam Mukhade <[email protected]>
  • Loading branch information
Shivam Mukhade committed Sep 7, 2021
1 parent 805be35 commit 8dfdac2
Show file tree
Hide file tree
Showing 8 changed files with 104 additions and 28 deletions.
10 changes: 0 additions & 10 deletions pkg/apis/operator/v1alpha1/tektonconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,6 @@ func (a Addon) IsEmpty() bool {
return reflect.DeepEqual(a, Addon{})
}

// Pipeline defines the field to customize Pipeline component
type Pipeline struct {
PipelineProperties `json:",inline"`
}

// Trigger defines the field to customize Trigger component
type Trigger struct {
TriggersProperties `json:",inline"`
}

type Config struct {
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
Expand Down
12 changes: 10 additions & 2 deletions pkg/apis/operator/v1alpha1/tektonpipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ func (tp *TektonPipeline) GetStatus() TektonComponentStatus {

// TektonPipelineSpec defines the desired state of TektonPipeline
type TektonPipelineSpec struct {
CommonSpec `json:",inline"`
PipelineProperties `json:",inline"`
CommonSpec `json:",inline"`
Pipeline `json:",inline"`
// Config holds the configuration for resources created by TektonPipeline
// +optional
Config Config `json:"config,omitempty"`
Expand All @@ -71,6 +71,14 @@ type TektonPipelineList struct {
Items []TektonPipeline `json:"items"`
}

// Pipeline defines the field to customize Pipeline component
type Pipeline struct {
PipelineProperties `json:",inline"`
// The params to customize different components of Pipelines
// +optional
Params []Param `json:"params,omitempty"`
}

// PipelineProperties defines customizable flags for Pipeline Component.
type PipelineProperties struct {
DisableAffinityAssistant *bool `json:"disable-affinity-assistant,omitempty"`
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/operator/v1alpha1/tektontrigger_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ type TektonTriggerList struct {
Items []TektonTrigger `json:"items"`
}

// Trigger defines the field to customize Trigger component
type Trigger struct {
TriggersProperties `json:",inline"`
}

// TriggersProperties defines the fields which are to be
// defined for triggers only if user pass them
type TriggersProperties struct {
Expand Down
7 changes: 6 additions & 1 deletion pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go

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

8 changes: 4 additions & 4 deletions pkg/reconciler/kubernetes/tektonconfig/pipeline/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func ensureTektonPipelineExists(clients op.TektonPipelineInterface, config *v1al
updated = true
}

if !reflect.DeepEqual(tpCR.Spec.PipelineProperties, config.Spec.Pipeline.PipelineProperties) {
tpCR.Spec.PipelineProperties = config.Spec.Pipeline.PipelineProperties
if !reflect.DeepEqual(tpCR.Spec.Pipeline, config.Spec.Pipeline) {
tpCR.Spec.Pipeline = config.Spec.Pipeline
updated = true
}

Expand Down Expand Up @@ -90,8 +90,8 @@ func ensureTektonPipelineExists(clients op.TektonPipelineInterface, config *v1al
CommonSpec: v1alpha1.CommonSpec{
TargetNamespace: config.Spec.TargetNamespace,
},
PipelineProperties: config.Spec.Pipeline.PipelineProperties,
Config: config.Spec.Config,
Pipeline: config.Spec.Pipeline,
Config: config.Spec.Config,
},
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/openshift/tektonconfig/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (oe openshiftExtension) Transformers(comp v1alpha1.TektonComponent) []mf.Tr
func (oe openshiftExtension) PreReconcile(ctx context.Context, tc v1alpha1.TektonComponent) error {

config := tc.(*v1alpha1.TektonConfig)
pipelineUpdated := openshiftPipeline.SetDefault(&config.Spec.Pipeline.PipelineProperties)
pipelineUpdated := openshiftPipeline.SetDefault(&config.Spec.Pipeline)
triggerUpdated := openshiftTrigger.SetDefault(&config.Spec.Trigger.TriggersProperties)
if pipelineUpdated || triggerUpdated {
if _, err := oe.operatorClientSet.OperatorV1alpha1().TektonConfigs().Update(ctx, config, v1.UpdateOptions{}); err != nil {
Expand Down
88 changes: 78 additions & 10 deletions pkg/reconciler/openshift/tektonpipeline/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ const (
// DefaultDisableAffinityAssistant is default value of disable affinity assistant flag
DefaultDisableAffinityAssistant = true
monitoringLabel = "openshift.io/cluster-monitoring=true"

enableMetricsKey = "enableMetrics"
enableMetricsDefaultValue = "true"
)

func OpenShiftExtension(ctx context.Context) common.Extension {
Expand All @@ -66,11 +69,20 @@ type openshiftExtension struct {
}

func (oe openshiftExtension) Transformers(comp v1alpha1.TektonComponent) []mf.Transformer {
return []mf.Transformer{
common.InjectLabelOnNamespace(monitoringLabel),
trns := []mf.Transformer{
occommon.ApplyCABundles,
occommon.RemoveRunAsUser(),
}

pipeline := comp.(*v1alpha1.TektonPipeline)

// Add monitoring label if metrics is enabled
value := findParam(pipeline.Spec.Params, enableMetricsKey)
if value == "" || value == "true" {
trns = append(trns, common.InjectLabelOnNamespace(monitoringLabel))
}

return trns
}
func (oe openshiftExtension) PreReconcile(ctx context.Context, tc v1alpha1.TektonComponent) error {
koDataDir := os.Getenv(common.KoEnvKey)
Expand Down Expand Up @@ -99,7 +111,7 @@ func (oe openshiftExtension) PreReconcile(ctx context.Context, tc v1alpha1.Tekto
}

tp := tc.(*v1alpha1.TektonPipeline)
if crUpdated := SetDefault(&tp.Spec.PipelineProperties); crUpdated {
if crUpdated := SetDefault(&tp.Spec.Pipeline); crUpdated {
if _, err := oe.operatorClientSet.OperatorV1alpha1().TektonPipelines().Update(ctx, tp, v1.UpdateOptions{}); err != nil {
return err
}
Expand All @@ -108,29 +120,85 @@ func (oe openshiftExtension) PreReconcile(ctx context.Context, tc v1alpha1.Tekto
return nil
}

func (oe openshiftExtension) PostReconcile(context.Context, v1alpha1.TektonComponent) error {
return nil
func (oe openshiftExtension) PostReconcile(ctx context.Context, comp v1alpha1.TektonComponent) error {
koDataDir := os.Getenv(common.KoEnvKey)
pipeline := comp.(*v1alpha1.TektonPipeline)

// Install monitoring if metrics is enabled
value := findParam(pipeline.Spec.Params, enableMetricsKey)

monitoringLocation := filepath.Join(koDataDir, "openshift-monitoring")
if err := common.AppendManifest(&oe.manifest, monitoringLocation); err != nil {
return err
}

trns, err := oe.manifest.Transform(
mf.InjectNamespace(pipeline.Spec.TargetNamespace),
mf.InjectOwner(pipeline),
)
if err != nil {
return err
}

if value == "true" {
return trns.Apply()
}
return trns.Delete()
}
func (oe openshiftExtension) Finalize(context.Context, v1alpha1.TektonComponent) error {
return nil
}

func SetDefault(properties *v1alpha1.PipelineProperties) bool {
func SetDefault(pipeline *v1alpha1.Pipeline) bool {

var updated = false

// Set default service account as pipeline
if properties.DefaultServiceAccount == "" {
properties.DefaultServiceAccount = common.DefaultSA
if pipeline.DefaultServiceAccount == "" {
pipeline.DefaultServiceAccount = common.DefaultSA
updated = true
}

// Set `disable-affinity-assistant` to true if not set in CR
// webhook will not set any value but by default in pipelines configmap it will be false
if properties.DisableAffinityAssistant == nil {
properties.DisableAffinityAssistant = ptr.Bool(DefaultDisableAffinityAssistant)
if pipeline.DisableAffinityAssistant == nil {
pipeline.DisableAffinityAssistant = ptr.Bool(DefaultDisableAffinityAssistant)
updated = true
}

// Add params with default values if not defined by user
var found = false
for i, p := range pipeline.Params {
if p.Name == enableMetricsKey {
found = true
// If the value set is invalid then set key to default value
// Not returning an error if the values is invalid as
// we validate in reconciler and this would affect the
// rest of the installation
if p.Value != "false" && p.Value != "true" {
pipeline.Params[i].Value = enableMetricsDefaultValue
updated = true
}
break
}
}

if !found {
pipeline.Params = append(pipeline.Params, v1alpha1.Param{
Name: enableMetricsKey,
Value: enableMetricsDefaultValue,
})
updated = true
}

return updated
}

func findParam(params []v1alpha1.Param, param string) string {
for _, p := range params {
if p.Name == param {
return p.Value
}
}
return ""
}

0 comments on commit 8dfdac2

Please sign in to comment.