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

use latest version SDK and go #63

Merged
merged 2 commits into from
Aug 30, 2021

Conversation

gthao313
Copy link
Member

@gthao313 gthao313 commented Jul 13, 2021

Issue number:
#52

Description of changes:

  • Building process uses old version SDK version, update makefile to use latest SDK verison.
  • Update go version

Testing done:
Building images

  1. Building a bottlerocket update operator image.
[ec2-user@ip-172-31-38-250 bottlerocket-update-operator]$ docker image ls
REPOSITORY                                             TAG                        IMAGE ID       CREATED              SIZE
update-operator-arm64                                  v0.1.4-c451ec2e            eed93f8bd863   19 seconds ago       35MB
<none>                                                 <none>                     4c52d26e902a   30 seconds ago       4.26GB
<none>                                                 <none>                     fe2681a37757   57 seconds ago       209kB

2.Push image to ECR Public, and deploy to an aarch64 bottlerocket host. and Extract logs to check if it run successfully

kubectl apply -f ./update-operator.yaml

kubectl label node $(kubectl get nodes -o jsonpath='{.items[*].metadata.name}') bottlerocket.aws/updater-interface-version=2.0.0
kubectl get nodes -o json | jq -C -S '.items | map(.metadata|{(.name): (.annotations|to_entries|map(select(.key|startswith("bottlerock
et")))|from_entries)}) | add'
{
  "ip-192-168-67-242.us-west-2.compute.internal": {
    "bottlerocket.aws/action-active": "reboot-update",
    "bottlerocket.aws/action-state": "ready",
    "bottlerocket.aws/action-wanted": "reboot-update",
    "bottlerocket.aws/update-available": "true"
  }
}

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@gthao313 gthao313 marked this pull request as ready for review July 13, 2021 17:08
Copy link
Contributor

@srgothi92 srgothi92 left a comment

Choose a reason for hiding this comment

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

Don't know anything about buildx plugin would let others review that part. I only reviewed the Go version change. Just confirming, Did you also repeated the testing after updating the Go version?

CHANGELOG.md Outdated Show resolved Hide resolved
@gthao313 gthao313 force-pushed the dockerfile-update branch 2 times, most recently from a563ca9 to f0aa42a Compare July 15, 2021 17:37
@gthao313
Copy link
Member Author

Push above remove change in Changelog according to @srgothi92 comment.

Copy link
Contributor

@srgothi92 srgothi92 left a comment

Choose a reason for hiding this comment

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

Version change LGTM.

Copy link
Contributor

@samuelkarp samuelkarp left a comment

Choose a reason for hiding this comment

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

I don't think we need this change; the SDK already has the ability to build binaries for the correct target architecture without involving buildx.

Have you run into problems using the SDK for that?

Makefile Outdated Show resolved Hide resolved
@gthao313
Copy link
Member Author

gthao313 commented Jul 29, 2021

Convert it to draft and will make sure if new change meets @samuelkarp 's expectation.

solution:
The arm image did run but metadata was not correct (expect arm but amd).
I was trying to use buildx as part of a multi-stage build to attach the correct metadata.

this change will make brupop use latest version SDK and
fix aarch64 building failure issue.
@gthao313 gthao313 changed the title support to build non-native images us latest version SDK Aug 24, 2021
@gthao313 gthao313 marked this pull request as ready for review August 24, 2021 01:53
@gthao313 gthao313 changed the title us latest version SDK use latest version SDK Aug 24, 2021
@gthao313 gthao313 changed the title use latest version SDK use latest version SDK and go Aug 24, 2021
Copy link
Contributor

@cbgbt cbgbt left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

Copy link

@WilboMo WilboMo left a comment

Choose a reason for hiding this comment

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

LGTM

@gthao313 gthao313 merged commit 547b9a2 into bottlerocket-os:develop Aug 30, 2021
@gthao313 gthao313 deleted the dockerfile-update branch August 31, 2021 23:47
@cbgbt cbgbt mentioned this pull request Sep 7, 2021
4 tasks
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.

5 participants