Skip to content

Support new files|volumes spec with score-go v1.10.0 #164

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Jun 10, 2025
Merged

Support new files|volumes spec with score-go v1.10.0 #164

merged 12 commits into from
Jun 10, 2025

Conversation

mathieu-benoit
Copy link
Contributor

@mathieu-benoit mathieu-benoit commented Apr 29, 2025 •
edited
Loading

Like it was done there score-spec/score-compose#302.

github.com/score-spec/score-go v1.10.0

score-spec/score-go#103 based on score-spec/spec#123 (comment)

This PR also makes sure that previous/old files|volumes spec is still working while we support the new spec.

Both examples working with this new version (and are not generating any diff with their manifests.yaml files):

Old files|volumes spec:

apiVersion: score.dev/v1b1
metadata:
  name: hello-world
containers:
  hello:
    image: .
    files:
      - target: /fileB.txt
        content: |
          I am ${metadata.name}
    volumes:
      - target: /data
        source: ${resources.data}
resources:
  data:
    type: volume

New files|volumes spec:

apiVersion: score.dev/v1b1
metadata:
  name: hello-world
containers:
  hello:
    image: .
    files:
      "/fileB.txt":
        content: |
          I am ${metadata.name}
    volumes:
      "/data":
        source: ${resources.data}
resources:
  data:
    type: volume

Also, just to keep in mind that with a previous version of score-k8s, here is the diff with this new implementation with the given example above:

42c42
<                       name: vol-0
---
>                       name: vol-bd47413b5c
44c44
<                       name: file-0
---
>                       name: file-ae7f6a5c41
47c47
<                   name: vol-0
---
>                   name: vol-bd47413b5c
52,53c52,53
<                     name: hello-world-hello-file-0
<                   name: file-0
---
>                     name: hello-world-hello-file-ae7f6a5c41
>                   name: file-ae7f6a5c41

@mathieu-benoit
github.com/score-spec/score-go v1.10.0
2ea7550 
Signed-off-by: Mathieu Benoit <mathieu-benoit@hotmail.fr>
@mathieu-benoit mathieu-benoit marked this pull request as draft April 29, 2025 20:22
@mathieu-benoit mathieu-benoit self-assigned this Apr 29, 2025
@mathieu-benoit
Add Makefile
f3d89e5 
Signed-off-by: Mathieu Benoit <mathieu-benoit@hotmail.fr>
@mathieu-benoit
Fix make build
6957ec2 
Signed-off-by: Mathieu Benoit <mathieu-benoit@hotmail.fr>
@astromechza
Copy link
Member

@mathieu-benoit do you need support here? Can I help on this one?

@astromechza astromechza mentioned this pull request May 20, 2025
Closed
@mathieu-benoit
Copy link
Contributor Author

Thanks, but still need to find time, hopefully I'll get this done this week.

@mathieu-benoit mathieu-benoit mentioned this pull request Jun 4, 2025
Merged
@mathieu-benoit mathieu-benoit marked this pull request as ready for review June 4, 2025 18:00
@mathieu-benoit
Copy link
Contributor Author

mathieu-benoit commented Jun 4, 2025 •
edited
Loading

@astromechza ready for your review please.

Couple of call out:

  • As an example vol-0 (len=8) will now become vol-65727a86f76df2a781b5e923a2046c9b536971f4a5a406f791ca1ffb22d5cbe6 (len=68) --> maybe we want to truncate here? Any tips if needed?
  • As an other example proj-vol-0 will stay like this, no sha here. Ok like this?

I still want to deploy some examples with score-k8s to make sure we can actually deploy these manifests in Kubernetes (to check if there is any constraint in name's length, etc.), but please in the meantime if you can review/comment it, it would be much appreciated. Thanks!

@astromechza
Copy link
Member

@mathieu-benoit yes I think you should truncate to first 8 characters of the vol or file hash.

astromechza
astromechza reviewed Jun 5, 2025
View reviewed changes
astromechza
astromechza reviewed Jun 5, 2025
View reviewed changes
astromechza
astromechza reviewed Jun 5, 2025
View reviewed changes
astromechza
astromechza reviewed Jun 5, 2025
View reviewed changes
astromechza
astromechza requested changes Jun 5, 2025
View reviewed changes
Copy link
Member

@astromechza astromechza left a comment

Choose a reason for hiding this comment

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

Truncate the hashes, use constants in the tests, and remove the score-k8s binary in the commit.

@mathieu-benoit
Delete score-k8s
1d7c59a 
Signed-off-by: Mathieu Benoit <mathieu-benoit@hotmail.fr>
@astromechza
Copy link
Member

@mathieu-benoit you'll need to allocate and then slice

func main() {
	hash := sha256.Sum256([]byte("ehllo"))
	fmt.Printf("file-%8x", hash[:5])
}

@mathieu-benoit
score-k8s binary in .gitignore
8f17b00 
Signed-off-by: Mathieu Benoit <mathieu-benoit@hotmail.fr>
@mathieu-benoit mathieu-benoit changed the title github.com/score-spec/score-go v1.10.0 Integrate new volumes and files spec with score-go v1.10.0 Jun 5, 2025
@mathieu-benoit
truncate hash
582e53e 
Signed-off-by: Mathieu Benoit <mathieu-benoit@hotmail.fr>
@mathieu-benoit
Fix unit tests in YAML formatting
672113b 
Signed-off-by: Mathieu Benoit <mathieu-benoit@hotmail.fr>
@mathieu-benoit mathieu-benoit changed the title Integrate new volumes and files spec with score-go v1.10.0 Support new files|volumes spec with score-go v1.10.0 Jun 5, 2025
@mathieu-benoit
Copy link
Contributor Author

Hash now truncated and score-k8s generate successfully tested and documented in the description of this PR.

Now working on the remaining unit tests failure which seems to be a flaky list ordering... stay tuned.

@mathieu-benoit
Copy link
Contributor Author

This is actually ready for the final review, please @astromechza.

Just ran 10+ times the unit tests locally and they were also successfull with the 2 last CI runs.

astromechza
astromechza approved these changes Jun 10, 2025
View reviewed changes
Copy link
Member

@astromechza astromechza left a comment

Choose a reason for hiding this comment

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

Thanks @mathieu-benoit! Great contribution :) I hope you enjoyed working more with Go for a change ;)

@mathieu-benoit mathieu-benoit merged commit a4e9514 into main Jun 10, 2025
8 checks passed
@mathieu-benoit mathieu-benoit deleted the score-go-1-10-0 branch June 10, 2025 13:50
@mathieu-benoit
Copy link
Contributor Author

I did and I do! Thanks for your support, great learnings here! More to come.

This was referenced Jun 10, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@astromechza astromechza astromechza approved these changes

Assignees

@mathieu-benoit mathieu-benoit

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mathieu-benoit @astromechza