Build Reasons and Changes diff #556

sukhil-suresh · 2020-11-23T15:36:26Z

The PR updates the prepare and rebase stages of the Build process to display the Build Reason and corresponding Change diff.

Sample output captured in this comment
Build Reason sort index is currently set to "TRIGGER,COMMIT,CONFIG,BUILDPACK,STACK"
Username was dropped from the TRIGGER message (at least for the first iteration) after discussion

djoyahoy · 2020-11-23T20:43:09Z

@sukhil-suresh is there an environment variable limit that would prevent use from storing all changes in the BUILD_CHANGES env var?

sukhil-suresh · 2020-11-23T21:10:14Z

@sukhil-suresh is there an environment variable limit that would prevent use from storing all changes in the BUILD_CHANGES env var?

Yes, there is a size limit on linux per environment variable and I believe for the combined block of env variables. I don't have a definitive number on hand; but based on google search it seems to be a 128KiB limit. It is also configurable.

The alternative considered was the Downward API Volume files but opted not to go down that path based on some very rough math of the changes value maxing out between 1500..2000 chars (at the moment)

But yes, still open to changing approach of passing the Changes value around

djoyahoy · 2020-11-23T21:14:39Z

@sukhil-suresh is there an environment variable limit that would prevent use from storing all changes in the BUILD_CHANGES env var?

Yes, there is a size limit on linux per environment variable and I believe for the combined block of env variables. I don't have a definitive number on hand; but based on google search it seems to be a 128KiB limit. It is also configurable.

The alternative considered was the Downward API Volume files but opted not to go down that path based on some very rough math of the changes value maxing out between 1500..2000 chars (at the moment)

But yes, still open to changing approach of passing the Changes value around

That makes sense, thanks!

sukhil-suresh · 2020-11-23T22:22:30Z

Captured Build reason(s) and Change diff for the different Build Reasons: CONFIG, TRIGGER, COMMIT, STACK, and BUILDPACK
The last one is for a combination of TRIGGER and BUILDPACK

CONFIG

$ kp image create test-image --tag sukhilsuresh/test-image --git https://github.com/paketo-buildpacks/samples --git-revision main --sub-path go/mod
Creating Image...
Image "test-image" created

$ kp build logs test-image
===> PREPARE
Build reason(s): CONFIG
CONFIG change:
	resources:
	  Limits: null
	  Requests: null
	source:
	  Blob: null
	-   Git: null
	+   Git:
	+     revision: 948b2eff6a21580a44a0f4d8c609a2af45359d41
	+     url: https://github.com/paketo-buildpacks/samples
	  Registry: null
	-   SubPath: ""
	+   SubPath: go/mod
Loading secret for "gcr.io" from secret "gcr" at location "/var/build-secrets/gcr"
....

TRIGGER

$ kp image trigger test-image
Triggered build for Image "test-image"

$ kp build logs test-image
===> PREPARE
Build reason(s): TRIGGER
TRIGGER change:
	A new build was manually triggered on Mon, 23 Nov 2020 16:16:26 -0500
Loading secret for "gcr.io" from secret "gcr" at location "/var/build-secrets/gcr"
...

COMMIT

$ kp build logs test-image
===> PREPARE
Build reason(s): COMMIT
COMMIT change:
	- 948b2eff6a21580a44a0f4d8c609a2af45359d41
	+ ec6f737864fa12374d5e8ee1db09058ab4e21ebf
Loading secret for "gcr.io" from secret "gcr" at location "/var/build-secrets/gcr"
...

STACK

$ kp clusterstack update full -b registry.pivotal.io/tbs-dependencies/build-full@sha256:e60c6717b2b6e8458f8f782c13fd2f60ef9e2f1f378fbb219be5da13538114aa -r registry.pivotal.io/tbs-dependencies/run-full@sha256:42841631725942db48b7ba8b788b97374a2ada34c84ee02ca5e02ef3d4b0dfca
Updating ClusterStack...
Uploading to 'gcr.io/cf-build-service-dev-219913/ssuresh/install'...
	Uploading 'gcr.io/cf-build-service-dev-219913/ssuresh/install/build@sha256:e60c6717b2b6e8458f8f782c13fd2f60ef9e2f1f378fbb219be5da13538114aa'
	Uploading 'gcr.io/cf-build-service-dev-219913/ssuresh/install/run@sha256:42841631725942db48b7ba8b788b97374a2ada34c84ee02ca5e02ef3d4b0dfca'


$ kp build logs test-image
===> REBASE
Build reason(s): STACK
STACK change:
	- RunImage: sha256:87302783be0a0cab9fde5b68c9954b7e9150ca0d514ba542e9810c3c6f2984ad
	+ RunImage: sha256:42841631725942db48b7ba8b788b97374a2ada34c84ee02ca5e02ef3d4b0dfca
Loading secret for "gcr.io" from secret "gcr" at location "/var/build-secrets/gcr"
...

BUILDPACK

$ kp clusterstore status default
Status:    Ready

BUILDPACKAGE ID                       VERSION    HOMEPAGE
paketo-buildpacks/procfile            2.0.2      https://github.com/paketo-buildpacks/procfile
tanzu-buildpacks/dotnet-core          0.0.6
tanzu-buildpacks/go                   1.0.5
tanzu-buildpacks/httpd                0.0.39
tanzu-buildpacks/java                 3.8.0      https://github.com/pivotal-cf/tanzu-java
tanzu-buildpacks/java-native-image    3.6.0      https://github.com/pivotal-cf/tanzu-java-native-image
tanzu-buildpacks/nginx                0.0.46
tanzu-buildpacks/nodejs               1.2.2
tanzu-buildpacks/php                  0.0.5

$ kp clusterstore add default -b registry.pivotal.io/tanzu-go-buildpack/go@sha256:ed99e73df43380c86c983e09d97151caf1952e8f423053a6e0a15de526515f4c
Adding to ClusterStore...
	Uploading 'gcr.io/cf-build-service-dev-219913/ssuresh/install/tanzu-buildpacks_go@sha256:ed99e73df43380c86c983e09d97151caf1952e8f423053a6e0a15de526515f4c'
	Added Buildpackage
ClusterStore "default" updated

$ kp clusterstore status default
Status:    Ready

BUILDPACKAGE ID                       VERSION    HOMEPAGE
paketo-buildpacks/procfile            2.0.2      https://github.com/paketo-buildpacks/procfile
tanzu-buildpacks/dotnet-core          0.0.6
tanzu-buildpacks/go                   1.0.5
tanzu-buildpacks/go                   1.0.7
tanzu-buildpacks/httpd                0.0.39
tanzu-buildpacks/java                 3.8.0      https://github.com/pivotal-cf/tanzu-java
tanzu-buildpacks/java-native-image    3.6.0      https://github.com/pivotal-cf/tanzu-java-native-image
tanzu-buildpacks/nginx                0.0.46
tanzu-buildpacks/nodejs               1.2.2
tanzu-buildpacks/php                  0.0.5

$ kp build logs test-image
===> PREPARE
Build reason(s): BUILDPACK
BUILDPACK change:
	- tanzu-buildpacks/go-build	0.0.22
	- tanzu-buildpacks/go-dist	0.1.0
	- tanzu-buildpacks/go-mod-vendor	0.0.24
	+ tanzu-buildpacks/go-build	0.0.23
	+ tanzu-buildpacks/go-dist	0.1.2
	+ tanzu-buildpacks/go-mod-vendor	0.0.25
Loading secret for "gcr.io" from secret "gcr" at location "/var/build-secrets/gcr"
...

Combination of TRIGER and BUILDPACK

$ kp build logs test-image
===> PREPARE
Build reason(s): TRIGGER,BUILDPACK
TRIGGER change:
	A new build was manually triggered on Mon, 23 Nov 2020 17:14:53 -0500
BUILDPACK change:
	- tanzu-buildpacks/go-build	0.0.23
	- tanzu-buildpacks/go-dist	0.1.2
	- tanzu-buildpacks/go-mod-vendor	0.0.25
	+ tanzu-buildpacks/go-build	0.0.22
	+ tanzu-buildpacks/go-dist	0.1.0
	+ tanzu-buildpacks/go-mod-vendor	0.0.24
Loading secret for "gcr.io" from secret "gcr" at location "/var/build-secrets/gcr"
...

sampeinado · 2020-11-24T16:37:58Z

This looks AWESOME !!

A few comments:

for trigger, it says "TRIGGER change:" which doesn't really make sense. Would it be possible to say just "TRIGGER"?
for the CONFIG change, how does the system decide how many additional lines to print other than the diffed lines?

sukhil-suresh · 2020-11-24T16:47:06Z

for trigger, it says "TRIGGER change:" which doesn't really make sense. Would it be possible to say just "TRIGGER"?

@sampeinado Assuming you were referring to a format similar to the one below? Yes, can make that change

===> PREPARE
Build reason(s): TRIGGER,BUILDPACK
TRIGGER: A new build was manually triggered on Mon, 23 Nov 2020 16:16:26 -0500
BUILDPACK change:
	- tanzu-buildpacks/go-build	0.0.23
	- tanzu-buildpacks/go-dist	0.1.2
	- tanzu-buildpacks/go-mod-vendor	0.0.25
	+ tanzu-buildpacks/go-build	0.0.22
	+ tanzu-buildpacks/go-dist	0.1.0
	+ tanzu-buildpacks/go-mod-vendor	0.0.24

sampeinado · 2020-11-24T16:54:42Z

yes exactly! I think that makes more sense

sukhil-suresh · 2020-11-24T17:11:42Z

@sampeinado

for the CONFIG change, how does the system decide how many additional lines to print other than the diffed lines?

For context...

$ kp build logs test-image
===> PREPARE
Build reason(s): CONFIG
CONFIG change:
	resources:
	  Limits: null
	  Requests: null
	source:
	  Blob: null
	-   Git: null
	+   Git:
	+     revision: 948b2eff6a21580a44a0f4d8c609a2af45359d41
	+     url: https://github.com/paketo-buildpacks/samples
	  Registry: null
	-   SubPath: ""
	+   SubPath: go/mod
Loading secret for "gcr.io" from secret "gcr" at location "/var/build-secrets/gcr"
...

I am assuming you are referring to the resources: being listed? I do see the oddity of it since it provides no additional context. I am not sure why that gets listed (in spite of the effort to suppress). Will look more into it.

About how the diff is generated? The Config objects are serialized to string and then compared. Test case that covers the CONFIG scenario.

Address change request on PR #556 (comment)

Address change request on PR #556 (comment) Co-authored-by: Elenore Bastian <ebastian@vmware.com>

Avoid showing irrelevant fields during diff Ref: #556 (comment) Co-authored-by: Elenore Bastian <ebastian@vmware.com>

sukhil-suresh · 2020-11-25T02:17:43Z

A few comments:

for trigger, it says "TRIGGER change:" which doesn't really make sense. Would it be possible to say just "TRIGGER"?

for the CONFIG change, how does the system decide how many additional lines to print other than the diffed lines?

Updated CONFIG, and TRIGGER change (based on @sampeinado comments)

CONFIG

$ kp image create test-image --tag sukhilsuresh/test-image --git https://github.com/paketo-buildpacks/samples --git-revision main --sub-path go/mod
Creating Image...
Image "test-image" created

$ kp build logs test-image
===> PREPARE
Build reason(s): CONFIG
CONFIG change:
	- source: {}
	+ source:
	+   git:
	+     revision: 948b2eff6a21580a44a0f4d8c609a2af45359d41
	+     url: https://github.com/paketo-buildpacks/samples
	+   subPath: go/mod
Loading secret for "gcr.io" from secret "gcr" at location "/var/build-secrets/gcr"
...

TRIGGER

$ kp image trigger test-image
Triggered build for Image "test-image"

$ kp build logs test-image
===> PREPARE
Build reason(s): TRIGGER
TRIGGER: A new build was manually triggered on Tue, 24 Nov 2020 21:12:39 -0500
Loading secret for "gcr.io" from secret "gcr" at location "/var/build-secrets/gcr"
...

pkg/reconciler/image/build_needed.go

pkg/reconciler/image/reconcile_build.go

pkg/buildchange/stack_change.go

pkg/buildchange/change_processor.go

pkg/buildchange/change_logger.go

pkg/buildchange/change_processor.go

pkg/buildchange/stack_change.go

- use only changes string - changes string is now parsed into an array of GenericChange object - changes on old/new fields are printed as is provided - multiple change errors are bunched and reported for ChangeProcessor Reason for the switch to a lightweight logger is captured in this issue comment: #556 (comment)

pkg/reconciler/image/build_required.go

Source: https://git.io/JklOZ https://git.io/JklOB Co-authored-by: Sukhil Suresh <ssukhil@vmware.com>

Update the `prepare` and `rebase` stages of the Build process to display the Build Reason and corresponding Change diff for a Build Co-authored-by: Elenore Bastian <ebastian@vmware.com>

- use only changes string - changes string is now parsed into an array of GenericChange object - changes on old/new fields are printed as is provided - multiple change errors are bunched and reported for ChangeProcessor Reason for the switch to a lightweight logger is captured in this issue comment: #556 (comment) Co-authored-by: Sukhil Suresh <ssukhil@vmware.com>

sukhil-suresh changed the title ~~Wip reason message diff~~ Reason message diff Nov 23, 2020

sukhil-suresh changed the title ~~Reason message diff~~ Build Reasons and Changes diff Nov 23, 2020

sukhil-suresh force-pushed the wip-reason-message-diff branch from a4b2562 to 383a06c Compare November 23, 2020 20:22

sukhil-suresh force-pushed the wip-reason-message-diff branch from 383a06c to 69baa7a Compare November 24, 2020 00:45

sukhil-suresh requested review from matthewmcnew, djoyahoy, tylerphelan, tomkennedy513, elenorebastian and aemengo November 24, 2020 00:53

sukhil-suresh linked an issue Nov 24, 2020 that may be closed by this pull request

Add reason-message and reasons diff to build logs #513

Closed

sukhil-suresh marked this pull request as ready for review November 24, 2020 00:57

sukhil-suresh force-pushed the wip-reason-message-diff branch from 69baa7a to 9d9a3f3 Compare November 24, 2020 01:03

sukhil-suresh marked this pull request as draft November 24, 2020 01:06

sukhil-suresh force-pushed the wip-reason-message-diff branch from 9d9a3f3 to 552762a Compare November 24, 2020 01:13

sukhil-suresh marked this pull request as ready for review November 24, 2020 01:18

sukhil-suresh added a commit that referenced this pull request Nov 24, 2020

Update logging of Trigger change

95cf552

Address change request on PR #556 (comment)

sukhil-suresh added a commit that referenced this pull request Nov 24, 2020

Update logging of Trigger change

f6ad291

Address change request on PR #556 (comment) Co-authored-by: Elenore Bastian <ebastian@vmware.com>

sukhil-suresh force-pushed the wip-reason-message-diff branch from 95cf552 to f6ad291 Compare November 24, 2020 17:49

sukhil-suresh added a commit that referenced this pull request Nov 25, 2020

Fix Config change diff

d6e0a33

Avoid showing irrelevant fields during diff Ref: #556 (comment) Co-authored-by: Elenore Bastian <ebastian@vmware.com>

sukhil-suresh force-pushed the wip-reason-message-diff branch from 9b929a0 to 88f3fca Compare November 25, 2020 17:35