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

fix: update the VERSION variable assignment method #1552

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

vprashar2929
Copy link
Collaborator

@vprashar2929 vprashar2929 commented Jun 18, 2024

This PR:

  • fixes the issue the VERSION variable assignment
    from ?= to := to ensure immediate evaluation of $(GIT_VERSION)

  • Use fetch_depth to fetch the tag info for computing the GIT_VERSION
    correctly while building Kepler images

  • Removes the VERSION argument from the Dockerfile

  • Removes the redundant bpftool COPY statement in build/Dockerfile

  • Addresses issue Version incorrectly reported in the container images #1540

Copy link

github-actions bot commented Jun 18, 2024

🤖 SeineSailor

Here is a concise summary of the pull request changes:

Summary: This PR updates the VERSION variable assignment method, renames it to IMAGE_VERSION, and refactors Dockerfile configurations to ensure immediate evaluation and consistency.

Key Modifications:

  1. Replaced ?= with := for immediate evaluation of $(GIT_VERSION).
  2. Renamed VERSION to IMAGE_VERSION and added it back to image.yml for the release workflow.
  3. Removed duplicate COPY statement from build/Dockerfile.
  4. Updated LDFLAGS and build arguments to use IMAGE_VERSION instead of VERSION.
  5. Added a conditional statement to the RUN make build command.

Impact: These changes fix issue #1540 and improve VERSION handling without affecting the external interface or behavior of the code. The signatures of exported functions and global data structures remain unaltered.

Observations: The changes are well-organized and address a specific issue. The use of := ensures immediate evaluation, which can prevent potential issues with variable assignment. The refactoring of Dockerfile configurations improves consistency and maintainability.

@sthaha
Copy link
Collaborator

sthaha commented Jun 18, 2024

@vprashar2929 , could you please explain why (instead of what) we are removing version?

Copy link
Collaborator

@sthaha sthaha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets make sure that this does not affect version in the release workflow.

@vprashar2929 vprashar2929 force-pushed the chore-dkf-make branch 3 times, most recently from f1120ec to 7983232 Compare June 19, 2024 11:32
@vprashar2929 vprashar2929 changed the title chore: remove version from build-arg fix: update the VERSION variable assignment method Jun 19, 2024
@vprashar2929 vprashar2929 marked this pull request as draft June 19, 2024 19:16
Makefile Show resolved Hide resolved
@vprashar2929 vprashar2929 force-pushed the chore-dkf-make branch 3 times, most recently from 07ce703 to cfccdb6 Compare June 20, 2024 05:59
@vprashar2929
Copy link
Collaborator Author

Kepler using the latest image:

kepler-latest-1  | I0620 06:22:45.350085  295372 gpu.go:42] Using dummy to obtain gpu power
kepler-latest-1  | I0620 06:22:45.351658  295372 exporter.go:100] Kepler running on version: v0.7.10-122-g9344179c
kepler-latest-1  | I0620 06:22:45.351682  295372 config.go:280] using gCgroup ID in the BPF program: false
kepler-latest-1  | I0620 06:22:45.351717  295372 config.go:282] kernel version: 6.8

Screenshot 2024-06-20 at 11 53 52 AM

Kepler using the dummy release image:

kepler-latest-1  | I0620 06:25:18.215265  298159 gpu.go:42] Using dummy to obtain gpu power
kepler-latest-1  | I0620 06:25:18.217287  298159 exporter.go:100] Kepler running on version: release-0.7.11
kepler-latest-1  | I0620 06:25:18.217362  298159 config.go:280] using gCgroup ID in the BPF program: false
kepler-latest-1  | I0620 06:25:18.217400  298159 config.go:282] kernel version: 6.8

Screenshot 2024-06-20 at 11 55 33 AM

@vprashar2929 vprashar2929 marked this pull request as ready for review June 20, 2024 06:25
This PR:
* fixes the issue the VERSION variable assignment
  from `?=` to `:=` to ensure immediate evaluation of `$(GIT_VERSION)`

* Use `fetch_depth` to fetch the tag info for computing the `GIT_VERSION`
  correctly while building Kepler images

* Removes the `VERSION` argument from the Dockerfile

* Removes the redundant bpftool COPY statement in build/Dockerfile

* Addresses issue sustainable-computing-io#1540

Signed-off-by: Vibhu Prashar <vibhu.sharma2929@gmail.com>
.github/workflows/image.yml Outdated Show resolved Hide resolved
Signed-off-by: Sunil Thaha <sthaha@redhat.com>
Signed-off-by: Sunil Thaha <sthaha@redhat.com>
@sthaha sthaha merged commit 0e22839 into sustainable-computing-io:main Jun 21, 2024
26 checks passed
vprashar2929 added a commit to vprashar2929/kepler that referenced this pull request Jun 21, 2024
Removes the `VERSION` build-arg from the docker compose files
as it was removed in sustainable-computing-io#1552

Signed-off-by: Vibhu Prashar <vibhu.sharma2929@gmail.com>
sthaha pushed a commit that referenced this pull request Jun 21, 2024
Removes the `VERSION` build-arg from the docker compose files
as it was removed in #1552

Signed-off-by: Vibhu Prashar <vibhu.sharma2929@gmail.com>
gdozortsev pushed a commit to gdozortsev/kepler that referenced this pull request Jun 21, 2024
Signed-off-by: Gabriela Dozortsev <gdozorts@redhat.com>

Update pkg/collector/stats/utils.go

Co-authored-by: Dave Tucker <dave@dtucker.co.uk>
Signed-off-by: Gabriela Dozortsev <81634840+gdozortsev@users.noreply.github.com>

Update pkg/collector/stats/utils.go

Co-authored-by: Dave Tucker <dave@dtucker.co.uk>
Signed-off-by: Gabriela Dozortsev <81634840+gdozortsev@users.noreply.github.com>

Update pkg/collector/stats/utils.go

Co-authored-by: Dave Tucker <dave@dtucker.co.uk>
Signed-off-by: Gabriela Dozortsev <81634840+gdozortsev@users.noreply.github.com>

fix: update the VERSION variable assignment method (sustainable-computing-io#1552)

* fix: update the VERSION variable assignment method

This PR:
* fixes the issue the VERSION variable assignment
  from `?=` to `:=` to ensure immediate evaluation of `$(GIT_VERSION)`

* Use `fetch_depth` to fetch the tag info for computing the `GIT_VERSION`
  correctly while building Kepler images

* Removes the `VERSION` argument from the Dockerfile

* Removes the redundant bpftool COPY statement in build/Dockerfile

* Addresses issue sustainable-computing-io#1540

Signed-off-by: Vibhu Prashar <vibhu.sharma2929@gmail.com>
Signed-off-by: Sunil Thaha <sthaha@redhat.com>
Co-authored-by: Sunil Thaha <sthaha@redhat.com>

chore: remove version from docker-compose (sustainable-computing-io#1562)

Removes the `VERSION` build-arg from the docker compose files
as it was removed in sustainable-computing-io#1552

Signed-off-by: Vibhu Prashar <vibhu.sharma2929@gmail.com>

Fixed format errors

Signed-off-by: Gabriela Dozortsev <gdozorts@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants