Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

committime exporter doesn't work with binary builds #317

Closed
jlozano-rh opened this issue Oct 21, 2021 · 12 comments · Fixed by #498
Closed

committime exporter doesn't work with binary builds #317

jlozano-rh opened this issue Oct 21, 2021 · 12 comments · Fixed by #498
Assignees
Labels
bd-1 compatibility A compatibility issue (versioning, exporter backends, etc.) mvr mvr as of 03/2022 needs investigation More research is necessary for this task, usually from developers of the project. Size Medium A medium amount of work

Comments

@jlozano-rh
Copy link

jlozano-rh commented Oct 21, 2021

In Red Hat we have customers that have all it's builds configured with Binary builds, that are similar to next:

source:
    type: Binary
strategy:
    type: Docker
    dockerStrategy:
      from:
        kind: ImageStreamTag
        namespace: openshift
        name: 'nodejs:12'
      dockerfilePath: openshift/Dockerfile

But the committime exporter is trying to find the next properties in builds triggered .spec.source.git.uri and .spec.revision.git.commit. These properties do not exist because the build is binary, so the committime exporter pod is recording the following log:

10-21-2021 17:26:14 WARNING  Build example-project/example-build-13 in app example-app is missing required attributes to collect data. Skipping.
10-21-2021 17:26:14 DEBUG    'NoneType' object has no attribute 'git'
Traceback (most recent call last):
  File "/opt/app-root/src/committime/collector_base.py", line 185, in get_metric_from_build
    commit_sha = build.spec.revision.git.commit
AttributeError: 'NoneType' object has no attribute 'git'
10-21-2021 17:26:14 DEBUG    Searching for builds with label: pelorus/app.name in namespace: example-project
10-21-2021 17:26:14 DEBUG    Repo URL for app example-app is currently None
10-21-2021 17:26:14 DEBUG    http://gitlab.company.com/example/app.git
10-21-2021 17:26:14 DEBUG    http://gitlab.company.com/example/app.git
10-21-2021 17:26:14 WARNING  Build example-project/example-app-2 in app example-app is missing required attributes to collect data. Skipping.
10-21-2021 17:26:14 DEBUG    'NoneType' object has no attribute 'imageDigest'

It would be a good idea to propose that the committime exporter code implement a way of doing things for binary builds.

@KevinMGranger KevinMGranger added compatibility A compatibility issue (versioning, exporter backends, etc.) needs investigation More research is necessary for this task, usually from developers of the project. labels Dec 21, 2021
@eformat
Copy link
Contributor

eformat commented Feb 14, 2022

Hi @KevinMGranger @jlozano-rh .. i dont know if any prior art/thinking has gone on with this one?

given that Binary Builds have none of the required information in OCP for git, i have a prototype / hack working that relies on annotating the Build from a pipeline (i'm using Tekton) but it could be any pipeline script really:

oc start-build $(params.APPLICATION_NAME) --from-archive=$(params.APPLICATION_NAME)-$(params.VERSION).tar.gz --follow --wait
LATEST_BUILD=$(oc get bc $(params.APPLICATION_NAME) --template='{{ .metadata.name }}-{{ .status.lastVersion }}')
oc annotate build/${LATEST_BUILD} --overwrite buildSpecSourceGitUri=$(params.GIT_URL) buildSpecRevisionGitCommit=$(params.GIT_COMMIT) buildSpecRevisionGitAuthorName="joe.blogs"

and then some minor modifications in the python collector_base.py

i would be interested if others have any ideas since binary builds are extremely common

@jlozano-rh
Copy link
Author

@eformat yes, in my case... I had to modify the BuildConfig object dynamically with Jenkins or Tekton adding the necessary data like git url, hash of last commit, etc.

And modify collector_base.py to be able to read that data from the BuildConfig. For example, I did this modifications:

    def get_metric_from_build(self, build, app, namespace, repo_url):
        try:

            ...

            if not repo_url:
                if build.spec.source.git:
                    repo_url = build.spec.source.git.uri
                elif build.metadata.annotations.pelorusgiturl:
                    repo_url = build.metadata.annotations.pelorusgiturl
                else:
                    repo_url = self._get_repo_from_build_config(build)

            metric.repo_url = repo_url

Or something like that.

@eformat
Copy link
Contributor

eformat commented Feb 15, 2022

ah yup .. me to. same approach. OK .. wonder if we go with that? and add instructions for the annotations for binary builds. @KevinMGranger - WDYT ?

@weshayutin weshayutin added the mvr mvr as of 03/2022 label Mar 17, 2022
@mpryc
Copy link
Collaborator

mpryc commented Apr 6, 2022

Related to #371

@mpryc mpryc self-assigned this Apr 6, 2022
@mpryc mpryc added the Size Medium A medium amount of work label Apr 6, 2022
@KevinMGranger
Copy link
Collaborator

@eformat we're thinking that the approach in #371 is probably best (have the resulting image have the commit ID and URL embedded).

Do either you or @jlozano-rh know of any use cases where that wouldn't be an option? If so, this is still a good idea and we'll probably use your PR.

@eformat
Copy link
Contributor

eformat commented Apr 11, 2022

hi @KevinMGranger .. #371 should work as well - ultimately the metadata for perlorus has to exist somewhere, so as labels on the OCI image also works.

The only difference i think would be if you wanted to capture start/end build times - which is easier on the BuildConfig, no reason you couldn't put that on the built image as well if desired, it would just be more custom labels perhaps.

@KevinMGranger
Copy link
Collaborator

For build times, I think we can get the metadata from the build pod itself, right? That's something we can investigate later.

@mpryc
Copy link
Collaborator

mpryc commented Apr 11, 2022

@jlozano-rh hello. The commit time exporter is expected to find out time of the repository commit and use it together with other metrics to measure "Lead time for change" - time from code committed to deployed to production.

I don't fully understand the requirement of monitoring binary build where the commit time is missing from provided build input. Could you please elaborate on how this is going to help in measuring lead time for change?

If the input for binary build provided is actually local git repository or compressed archive of git repository then it looks like it could potentially work ?

Maybe you could describe a bit more about your use case and how the build and deployment happens, e.g. is the Dockerfile provided in the build stored in a repo and why it's passed from local box rather then in a pipeline where changes are stored outside of mechanism to track historical commits.

@jlozano-rh
Copy link
Author

@mpryc I understand the point. I have no access anymore to the customer resources, but I remember something like this: the customer has a pipeline that have tasks in sequence: checkout, test, update-file, binary-build, deploy. So the binary-build occurs inside the pipeline through a Dockerfile.

I think like @KevinMGranger... maybe we can take the time from the build pod, for example: pod.metadata.creationTimestamp or something like that.

@KevinMGranger
Copy link
Collaborator

We need the time of the commit, not the time of the build. Is the checkout given as the input to the binary-build? If so, then we can use that.

@jlozano-rh
Copy link
Author

@KevinMGranger yes, the entire git repo directory (including the .git) is passed to binary-build task. So I think that we can do something like this:

import git
from datetime import datetime

repo = git.Repo("/path/to/git-repository")
last_commit = repo.head.commit
timestamp = datetime.fromtimestamp(last_commit.committed_date)
print(timestamp)

NOTE: I'm using gitpython dependency.

@KevinMGranger
Copy link
Collaborator

Unfortunately, it's not possible to inspect the contents of the binary build input. The only option we have is inspecting manual labels, or having some other work done in the build process.

We're looking into alternate solutions right now, but the manual labelling seems to be an okay solution.

mpryc added a commit to mpryc/pelorus that referenced this issue Jun 10, 2022
Change to resolve dora-metrics#317.

The idea is taken from PR dora-metrics#381, however the implementation
allows to have minimal required annotations to calculate
commit time. It also allows to define custom annotations.

Co-authored-by: Mike Hepburn <eformat@gmail.com>
mpryc added a commit to mpryc/pelorus that referenced this issue Jun 10, 2022
Change to resolve dora-metrics#317.

The idea is taken from PR dora-metrics#381, however the implementation
allows to have minimal required annotations to calculate
commit time. It also allows to define custom annotations.

Co-authored-by: Mike Hepburn <eformat@gmail.com>
mpryc added a commit to mpryc/pelorus that referenced this issue Jun 10, 2022
Change to resolve dora-metrics#317.

The idea is taken from PR dora-metrics#381, however the implementation
allows to have minimal required annotations to calculate
commit time. It also allows to define custom annotations.

Co-authored-by: Mike Hepburn <eformat@gmail.com>
mpryc added a commit to mpryc/pelorus that referenced this issue Jun 15, 2022
Change to resolve dora-metrics#317.

The idea is taken from PR dora-metrics#381, however the implementation
allows to have minimal required annotations to calculate
commit time. It also allows to define custom annotations.

Co-authored-by: Mike Hepburn <eformat@gmail.com>
mpryc added a commit to mpryc/pelorus that referenced this issue Jun 21, 2022
Change to resolve dora-metrics#317.

The idea is taken from PR dora-metrics#381, however the implementation
allows to have minimal required annotations to calculate
commit time. It also allows to define custom annotations.

Co-authored-by: Mike Hepburn <eformat@gmail.com>
weshayutin pushed a commit that referenced this issue Jun 21, 2022
Change to resolve #317.

The idea is taken from PR #381, however the implementation
allows to have minimal required annotations to calculate
commit time. It also allows to define custom annotations.

Co-authored-by: Mike Hepburn <eformat@gmail.com>

Co-authored-by: Mike Hepburn <eformat@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bd-1 compatibility A compatibility issue (versioning, exporter backends, etc.) mvr mvr as of 03/2022 needs investigation More research is necessary for this task, usually from developers of the project. Size Medium A medium amount of work
Projects
None yet
5 participants