Skip to content

Commit

Permalink
Refactors update logic for TektonPipeline reconciler
Browse files Browse the repository at this point in the history
Initially in pipeline installerset the annotation `lastAppliedHash`
of TektonPipeline spec was stored instead of hash of spec of
TektonInstallerSet

Hence this patch stores the hash of spec of TektonInstallerSet
as annotations to check if an upgrade is required for an installerset

Signed-off-by: Puneet Punamiya <[email protected]>
  • Loading branch information
PuneetPunamiya authored and tekton-robot committed Jun 17, 2022
1 parent 2253a3c commit 07c674a
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 36 deletions.
31 changes: 19 additions & 12 deletions pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/tektoncd/operator/pkg/reconciler/common"
installer "github.com/tektoncd/operator/pkg/reconciler/common/tektoninstallerset"
"github.com/tektoncd/operator/pkg/reconciler/kubernetes/tektoninstallerset"
"github.com/tektoncd/operator/pkg/reconciler/shared/hash"
tisBuilder "github.com/tektoncd/operator/pkg/reconciler/shared/tektoninstallerset"
"go.uber.org/zap"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -224,11 +223,12 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tp *v1alpha1.TektonPipel
// of TektonPipeline is changed by checking hash stored as annotation on
// TektonInstallerSet with computing new hash of TektonPipeline Spec

// Hash of TektonPipeline Spec
expectedSpecHash, err := hash.Compute(tp.Spec)
// Hash of TektonInstallerSet Spec
newIs, err := r.generateInstallerSet(ctx, tp)
if err != nil {
return err
}
expectedSpecHash := newIs.Annotations[v1alpha1.LastAppliedHashKey]

// spec hash stored on installerSet
lastAppliedHash := installedTIS.GetAnnotations()[v1alpha1.LastAppliedHashKey]
Expand Down Expand Up @@ -313,6 +313,20 @@ func (r *Reconciler) updateTektonPipelineStatus(ctx context.Context, tp *v1alpha

func (r *Reconciler) createInstallerSet(ctx context.Context, tp *v1alpha1.TektonPipeline) (*v1alpha1.TektonInstallerSet, error) {

generatedIs, err := r.generateInstallerSet(ctx, tp)
if err != nil {
return nil, err
}

createdIs, err := tisBuilder.Create(ctx, generatedIs)
if err != nil {
return nil, err
}

return createdIs, nil
}

func (r *Reconciler) generateInstallerSet(ctx context.Context, tp *v1alpha1.TektonPipeline) (*v1alpha1.TektonInstallerSet, error) {
// Creates a new default installer
dis := installer.NewDefaultInstaller()

Expand All @@ -337,19 +351,12 @@ func (r *Reconciler) createInstallerSet(ctx context.Context, tp *v1alpha1.Tekton
// Adds the annotations to the installer
dis.AddAnnotationsKeyVal(v1alpha1.TargetNamespaceKey, tp.Spec.TargetNamespace)

// Hash of TektonPipeline Spec
specHash, err := hash.Compute(tp.Spec)
generatedIs, err := tisBuilder.GenerateInstallerSetWithPrefixName(ctx, dis, v1alpha1.PipelineResourceName)
if err != nil {
return nil, err
}
dis.AddAnnotationsKeyVal(v1alpha1.LastAppliedHashKey, specHash)

// Creates installer set with generate name
createdIs, err := tisBuilder.CreateInstallerSetWithGenerateName(ctx, dis, v1alpha1.PipelineResourceName)
if err != nil {
return nil, err
}
return createdIs, nil
return generatedIs, nil
}

func (r *Reconciler) targetNamespaceCheck(ctx context.Context, tp *v1alpha1.TektonPipeline) error {
Expand Down
71 changes: 51 additions & 20 deletions pkg/reconciler/shared/tektoninstallerset/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,38 +22,46 @@ import (
mf "github.com/manifestival/manifestival"
"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
versionedClients "github.com/tektoncd/operator/pkg/client/clientset/versioned/typed/operator/v1alpha1"
"github.com/tektoncd/operator/pkg/reconciler/shared/hash"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// Creates the installerset using name
func CreateInstallerSetWithName(ctx context.Context, ci ComponentInstaller, name string) (*v1alpha1.TektonInstallerSet, error) {
// Generates the installerset with name
func GenerateInstallerSetWithName(ctx context.Context, ci ComponentInstaller, name string) (*v1alpha1.TektonInstallerSet, error) {
newISM := newTisMetaWithName(name)

return createInstallerSet(ctx, ci, newISM)
return generateInstallerSet(ctx, ci, newISM)
}

// Creates the installerset using generate name
func CreateInstallerSetWithGenerateName(ctx context.Context, ci ComponentInstaller, namePrefix string) (*v1alpha1.TektonInstallerSet, error) {
// Generates the installerset with prefix name
func GenerateInstallerSetWithPrefixName(ctx context.Context, ci ComponentInstaller, namePrefix string) (*v1alpha1.TektonInstallerSet, error) {
newISM := newTisMetaWithGenerateName(namePrefix)
return createInstallerSet(ctx, ci, newISM)
}

// Create the installerset
func createInstallerSet(ctx context.Context, ci ComponentInstaller, tis *tisMeta) (*v1alpha1.TektonInstallerSet, error) {
client := getTektonInstallerSetClient()
return createInstallerSetWithClient(ctx, client, ci, tis)
return generateInstallerSet(ctx, ci, newISM)
}

func createInstallerSetWithClient(ctx context.Context, client versionedClients.TektonInstallerSetInterface, ci ComponentInstaller, tis *tisMeta) (*v1alpha1.TektonInstallerSet, error) {
// Generates the installerset without applying on the cluster
func generateInstallerSet(ctx context.Context, ci ComponentInstaller, tis *tisMeta) (*v1alpha1.TektonInstallerSet, error) {
tis.config(ctx, ci)

manifest, err := ci.GetManifest(ctx)
if err != nil {
return nil, err
}

is := makeInstallerSet(manifest, tis)
is, err := makeInstallerSet(manifest, tis)
if err != nil {
return nil, err
}

return is, nil
}

// Creates the installerset on the cluster
func Create(ctx context.Context, is *v1alpha1.TektonInstallerSet) (*v1alpha1.TektonInstallerSet, error) {
client := getTektonInstallerSetClient()
return createWithClient(ctx, client, is)
}

func createWithClient(ctx context.Context, client versionedClients.TektonInstallerSetInterface, is *v1alpha1.TektonInstallerSet) (*v1alpha1.TektonInstallerSet, error) {
createdIs, err := client.Create(ctx, is, metav1.CreateOptions{})
if err != nil {
return nil, err
Expand All @@ -62,17 +70,40 @@ func createInstallerSetWithClient(ctx context.Context, client versionedClients.T
return createdIs, nil
}

func makeInstallerSet(manifest *mf.Manifest, mt *tisMeta) *v1alpha1.TektonInstallerSet {
return &v1alpha1.TektonInstallerSet{
func makeInstallerSet(manifest *mf.Manifest, mt *tisMeta) (*v1alpha1.TektonInstallerSet, error) {

is := &v1alpha1.TektonInstallerSet{
ObjectMeta: metav1.ObjectMeta{
Name: mt.Name,
GenerateName: mt.GenerateName,
Labels: mt.Labels,
Annotations: mt.Annotations,
OwnerReferences: mt.OwnerReferences,
},
Spec: v1alpha1.TektonInstallerSetSpec{
Manifests: manifest.Resources(),
},
Spec: installerSpec(manifest),
}

specHash, err := getHash(is.Spec)
if err != nil {
return nil, err
}
is.Annotations[v1alpha1.LastAppliedHashKey] = specHash

return is, nil
}

// Returns the spec of Installerset
func installerSpec(manifest *mf.Manifest) v1alpha1.TektonInstallerSetSpec {
return v1alpha1.TektonInstallerSetSpec{
Manifests: manifest.Resources(),
}
}

// Computes the hash using spec of TektonInstallerSet
func getHash(spec v1alpha1.TektonInstallerSetSpec) (string, error) {
specHash, err := hash.Compute(spec)
if err != nil {
return "", err
}
return specHash, nil
}
26 changes: 22 additions & 4 deletions pkg/reconciler/shared/tektoninstallerset/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,21 @@ func TestCreateInstallerset(t *testing.T) {

newISM := newTisMetaWithName("pipeline")

createdIs, err := createInstallerSetWithClient(context.Background(), client, di, newISM)
generateIs, err := generateInstallerSet(context.Background(), di, newISM)
assert.NilError(t, err)

createdIs, err := createWithClient(context.Background(), client, generateIs)
assert.Equal(t, err, nil)

labels := map[string]string{v1alpha1.CreatedByKey: "pipeline"}
annotations := map[string]string{"ns": "tekton-pipelines"}

specHash, err := getHash(installerSpec(&manifest))
assert.NilError(t, err)

annotations := map[string]string{
"ns": "tekton-pipelines",
"operator.tekton.dev/last-applied-hash": specHash,
}

expectedIs := &v1alpha1.TektonInstallerSet{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -88,10 +98,18 @@ func TestMakeInstallerset(t *testing.T) {
tis.Labels = map[string]string{v1alpha1.CreatedByKey: "pipeline"}
tis.Annotations = map[string]string{"ns": "tekton-pipelines"}

actual := makeInstallerSet(&manifest, tis)
actual, err := makeInstallerSet(&manifest, tis)
assert.NilError(t, err)

labels := map[string]string{v1alpha1.CreatedByKey: "pipeline"}
annotations := map[string]string{"ns": "tekton-pipelines"}

specHash, err := getHash(installerSpec(&manifest))
assert.NilError(t, err)

annotations := map[string]string{
"ns": "tekton-pipelines",
"operator.tekton.dev/last-applied-hash": specHash,
}

expected := &v1alpha1.TektonInstallerSet{
ObjectMeta: metav1.ObjectMeta{
Expand Down

0 comments on commit 07c674a

Please sign in to comment.