Skip to content

Commit

Permalink
Fix chains secret getting overwritten on upgrade
Browse files Browse the repository at this point in the history
This will fix chains secret getting overwritten over upgrade

Now secret has been moved out to a seprate installerset and
this installerset will not be removed and changed over upgrade.

This installlerset will only be deleted if the namespace
of the tektchains gets changed.
  • Loading branch information
piyush-garg committed Jun 27, 2022
1 parent 858cacc commit bea4b18
Showing 1 changed file with 109 additions and 12 deletions.
121 changes: 109 additions & 12 deletions pkg/reconciler/kubernetes/tektonchain/tektonchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ type Reconciler struct {
var _ tektonchainreconciler.Interface = (*Reconciler)(nil)
var _ tektonchainreconciler.Finalizer = (*Reconciler)(nil)

const createdByValue = "TektonChain"
const (
createdByValue = "TektonChain"
secretChainInstallerset = "chain-secret"
)

var (
ls = metav1.LabelSelector{
Expand All @@ -66,6 +69,12 @@ var (
v1alpha1.InstallerSetType: v1alpha1.ChainResourceName,
},
}
secretLs = metav1.LabelSelector{
MatchLabels: map[string]string{
v1alpha1.CreatedByKey: createdByValue,
v1alpha1.InstallerSetType: secretChainInstallerset,
},
}
)

// ReconcileKind compares the actual state with the desired, and attempts to
Expand Down Expand Up @@ -116,7 +125,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonChain
// Mark PreReconcile Complete
tc.Status.MarkPreReconcilerComplete()

// Check if a Tekton InstallerSet already exists, if not then create one
// Check if a Tekton Chains InstallerSet already exists, if not then create one
labelSelector, err := common.LabelSelector(ls)
if err != nil {
return err
Expand All @@ -136,7 +145,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonChain
return r.updateTektonChainStatus(tc, createdIs)
}

// If exists, then fetch the InstallerSet
// If exists, then fetch the TektonChain InstallerSet
installedTIS, err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
Get(ctx, existingInstallerSet, metav1.GetOptions{})
if err != nil {
Expand All @@ -154,9 +163,9 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonChain
installerSetTargetNamespace := installedTIS.Annotations[v1alpha1.TargetNamespaceKey]
installerSetReleaseVersion := installedTIS.Labels[v1alpha1.ReleaseVersionKey]

// Check if TargetNamespace of existing TektonInstallerSet is same as expected
// Check if Release Version in TektonInstallerSet is same as expected
// If any of the above things is not same then delete the existing TektonInstallerSet
// Check if TargetNamespace of existing TektonChainInstallerSet is same as expected
// Check if Release Version in TektonChainInstallerSet is same as expected
// If any of the above things is not same then delete the existing TektonChainInstallerSet
// and create a new with expected properties

if installerSetTargetNamespace != tc.Spec.TargetNamespace || installerSetReleaseVersion != r.operatorVersion {
Expand Down Expand Up @@ -184,7 +193,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonChain
} else {
// If target namespace and version are not changed then check if Chain
// spec is changed by checking hash stored as annotation on
// TektonInstallerSet with computing new hash of TektonChain Spec
// TektonChainInstallerSet with computing new hash of TektonChain Spec

// Hash of TektonChain Spec
expectedSpecHash, err := hash.Compute(tc.Spec)
Expand All @@ -209,6 +218,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonChain
if tc.Spec.GetTargetNamespace() == pipelineNamespace {
manifest = manifest.Filter(mf.Not(mf.ByKind("Namespace")))
}
// remove secret from this installerset as it will not be deleted on upgrade
manifest = manifest.Filter(mf.Not(mf.ByKind("Secret")))
if err := r.transform(ctx, &manifest, tc); err != nil {
logger.Error("manifest transformation failed: ", err)
return err
Expand All @@ -233,6 +244,64 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonChain
}
}

// Check if a Tekton Chains Secret InstallerSet already exists, if not then create one
secretLabelSelector, err := common.LabelSelector(secretLs)
if err != nil {
return err
}
existingSecretInstallerSet, err := tektoninstallerset.CurrentInstallerSetName(ctx, r.operatorClientSet, secretLabelSelector)
if err != nil {
return err
}
if existingSecretInstallerSet == "" {
tc.Status.MarkInstallerSetNotAvailable("Chain Secret InstallerSet not available")
createdIs, err := r.createSecretInstallerSet(ctx, tc)
if err != nil {
return err
}
return r.updateTektonChainStatus(tc, createdIs)
}

// If exists, then fetch the TektonChainSecret InstallerSet
installedSecretTIS, err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
Get(ctx, existingSecretInstallerSet, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
createdIs, err := r.createSecretInstallerSet(ctx, tc)
if err != nil {
return err
}
return r.updateTektonChainStatus(tc, createdIs)
}
logger.Error("failed to get InstallerSet: %s", err)
return err
}

secretInstallerSetTargetNamespace := installedSecretTIS.Annotations[v1alpha1.TargetNamespaceKey]
// if the namespace has been changed for chainsCR, then deleted the secret
if secretInstallerSetTargetNamespace != tc.Spec.TargetNamespace {
// Delete the existing TektonChainSecretInstallerSet
err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
Delete(ctx, existingSecretInstallerSet, metav1.DeleteOptions{})
if err != nil {
logger.Error("failed to delete TektonChainSecret InstallerSet: %s", err)
return err
}

// Make sure the TektonInstallerSet is deleted
_, err = r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
Get(ctx, existingSecretInstallerSet, metav1.GetOptions{})
if err == nil {
tc.Status.MarkNotReady("Waiting for previous installer set to get deleted")
return v1alpha1.REQUEUE_EVENT_AFTER
}
if !apierrors.IsNotFound(err) {
logger.Error("failed to get InstallerSet: %s", err)
return err
}
return nil
}

// Mark InstallerSetAvailable
tc.Status.MarkInstallerSetAvailable()

Expand Down Expand Up @@ -336,11 +405,17 @@ func (r *Reconciler) createInstallerSet(ctx context.Context, tc *v1alpha1.Tekton
if tc.Spec.GetTargetNamespace() == pipelineNamespace {
manifest = manifest.Filter(mf.Not(mf.ByKind("Namespace")))
}

// remove secret from this installerset as it will not be deleted on upgrade
manifest = manifest.Filter(mf.Not(mf.ByKind("Secret")))
if err := r.transform(ctx, &manifest, tc); err != nil {
tc.Status.MarkNotReady("transformation failed: " + err.Error())
return nil, err
}

// generate installer set
tis := makeInstallerSet(tc, manifest, v1alpha1.ChainResourceName, r.operatorVersion)

// compute the hash of tektonchain spec and store as an annotation
// in further reconciliation we compute hash of tc spec and check with
// annotation, if they are same then we skip updating the object
Expand All @@ -349,9 +424,32 @@ func (r *Reconciler) createInstallerSet(ctx context.Context, tc *v1alpha1.Tekton
if err != nil {
return nil, err
}
tis.Annotations[v1alpha1.LastAppliedHashKey] = specHash

// create installer set
createdIs, err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
Create(ctx, tis, metav1.CreateOptions{})
if err != nil {
return nil, err
}
return createdIs, nil
}

func (r *Reconciler) createSecretInstallerSet(ctx context.Context, tc *v1alpha1.TektonChain) (*v1alpha1.TektonInstallerSet, error) {

manifest := r.manifest
// filter only secret for this installerset as this needs
// to be restored over upgrade
manifest = manifest.Filter(mf.ByKind("Secret"))
if err := r.transform(ctx, &manifest, tc); err != nil {
tc.Status.MarkNotReady("transformation failed: " + err.Error())
return nil, err
}

// generate installer set
tis := makeInstallerSet(tc, manifest, secretChainInstallerset, r.operatorVersion)

// create installer set
tis := makeInstallerSet(tc, manifest, specHash, r.operatorVersion)
createdIs, err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
Create(ctx, tis, metav1.CreateOptions{})
if err != nil {
Expand All @@ -360,19 +458,18 @@ func (r *Reconciler) createInstallerSet(ctx context.Context, tc *v1alpha1.Tekton
return createdIs, nil
}

func makeInstallerSet(tc *v1alpha1.TektonChain, manifest mf.Manifest, tdSpecHash, releaseVersion string) *v1alpha1.TektonInstallerSet {
func makeInstallerSet(tc *v1alpha1.TektonChain, manifest mf.Manifest, installerSetType, releaseVersion string) *v1alpha1.TektonInstallerSet {
ownerRef := *metav1.NewControllerRef(tc, tc.GetGroupVersionKind())
return &v1alpha1.TektonInstallerSet{
ObjectMeta: metav1.ObjectMeta{
GenerateName: fmt.Sprintf("%s-", v1alpha1.ChainResourceName),
GenerateName: fmt.Sprintf("%s-", installerSetType),
Labels: map[string]string{
v1alpha1.CreatedByKey: createdByValue,
v1alpha1.ReleaseVersionKey: releaseVersion,
v1alpha1.InstallerSetType: v1alpha1.ChainResourceName,
v1alpha1.InstallerSetType: installerSetType,
},
Annotations: map[string]string{
v1alpha1.TargetNamespaceKey: tc.Spec.TargetNamespace,
v1alpha1.LastAppliedHashKey: tdSpecHash,
},
OwnerReferences: []metav1.OwnerReference{ownerRef},
},
Expand Down

0 comments on commit bea4b18

Please sign in to comment.