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

Update Notary framework to use theupdateframework #9

Closed
wants to merge 1 commit into from

Conversation

odidev
Copy link

@odidev odidev commented Jul 19, 2018

  • Update Script to use the new officially supported framework.
  • Update script to fetch the latest code and build binaries according to the host.

The changes are verified on both amd64 & arm64v8 architectures.
The script generated new static binaries for both architectures and docker was also running fine.

By taking this change, Notary can be extended to arm64v8 architecture as well.

@odidev odidev mentioned this pull request Jul 20, 2018
@odidev odidev mentioned this pull request Aug 23, 2018
@odidev
Copy link
Author

odidev commented Aug 27, 2018

Ping @cyli @NathanMcCauley

update.sh Outdated
@@ -1,5 +1,7 @@
#!/bin/bash

# Notary has moved to theupdateframework, so use new base.
Copy link
Member

Choose a reason for hiding this comment

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

I dont think we need to add this comment.

@odidev
Copy link
Author

odidev commented Aug 29, 2018

Thanks for the review @justincormack
I have updated this PR with the suggested modifications 😄

Regards,

@odidev
Copy link
Author

odidev commented Oct 12, 2018

Hi Guys,

Is there any update w.r.t. work in the direction of support for ARM64?

I would be happy to assist if something is required from my side to get Notary in the official support list of arm64v8

Regards,

@lag-linaro
Copy link
Contributor

+1 for this.

Please consider it for merge.

update.sh Outdated
mkdir -p $GOPATH/src/github.com/theupdateframework
cd $GOPATH/src/github.com/theupdateframework/
git clone https://github.com/theupdateframework/notary

Copy link
Member

Choose a reason for hiding this comment

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

You can't just clone the master branch here. The current assumption is that this is manually built from the right checkout.

Copy link
Author

Choose a reason for hiding this comment

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

So this means that the user should manually checkout https://github.com/theupdateframework/notary and then build notary code?

Actually, I tried to automate this by fetching all the dependencies for the user who wants to generate binaries for different architectures.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that really is not helpful for this repo, that workflow is not going to work, and if it did it would need all the architectures committed as different binaries. The proposal in docker-library/official-images#4718 is that we do release tarballs for more architectures, and use those for building official images.

Copy link
Author

Choose a reason for hiding this comment

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

Okay got you 😀
Please correct me if I am wrong.
It is sufficient to just update path in this update.sh to use theupdateframework.
And then you will be releasing binaries for ARM64 in the form of tarballs.

Regards,

@odidev
Copy link
Author

odidev commented Oct 26, 2018

Hi @justincormack

It has been a while, should I update the commit with the changes (just remove clone path) we were discussing or is there any other change which I should commit?

Regards,

- Update Script to use the new officially supported framework.
- Update script to fetch the latest code and build binaries
  according to the host.

Signed-off-by: Odidev <[email protected]>
@odidev
Copy link
Author

odidev commented Oct 29, 2018

@justincormack
As suggested by you, I have updated the commit in this PR for the quick resolution.
Please have a look and consider this PR for merge so that we can notary officially running over ARM64.

Regards,

@justincormack
Copy link
Member

This is fine, but it makes no difference to having official ARM64 support.

@odidev
Copy link
Author

odidev commented Oct 30, 2018

Yes, I agree with your concern.
But now a person can compile notary to generate binaries for AMD64 & ARM64 architectures.

However, for the official support, a PR will be required at https://github.com/docker-library/official-images/blob/master/library/notary
Can you handle this at your end?

After this, you will have to upload tarball packages for multiple architectures as mentioned by you in the earlier discussion.

Regards,

@lag-linaro
Copy link
Contributor

The update.sh script this PR adapts was made redundant by #6 and removed in #12 .

AArch64 support should be fixed by #10 and #11.

@odidev
Copy link
Author

odidev commented Nov 5, 2018

This is nice update @lag-linaro .
Now let's see when can we expect official AARCH64 support for Notary in Docker Hub.

@lag-linaro
Copy link
Contributor

Sure. Looks like the Official Images guys are happy with the current solution.

Would you mind closing this PR please? Since I believe that it no longer appropriate.

@odidev
Copy link
Author

odidev commented Nov 14, 2018

Hi @lag-linaro
Do you have any update regarding official support for AARCH64 in Notary?
Because it will require a PR at Docker Official-Images repository too.

Regards,

@lag-linaro
Copy link
Contributor

@odidev just waiting for @justincormack to review #10

@odidev
Copy link
Author

odidev commented Nov 26, 2018

Closing this ticket as AARCH64 on Notary is now an official part of Docker-Hub.
Thanks to all for their support.

@odidev odidev closed this Nov 26, 2018
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