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.

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

Bump Terraform from 0.11.7 to 0.11.8 #218

Merged
merged 2 commits into from
Sep 17, 2018
Merged

Bump Terraform from 0.11.7 to 0.11.8 #218

merged 2 commits into from
Sep 17, 2018

Conversation

wking
Copy link
Member

@wking wking commented Sep 6, 2018 •
edited
Loading

0.11.8 was cut on 2018-08-15. We'll need to wait until @sallyom pushes a 0.11.8 version of her image before the tf-fmt.sh change works.

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 6, 2018
@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 6, 2018
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 7, 2018
@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 11, 2018
@wking
Copy link
Member Author

wking commented Sep 11, 2018

The smoke error included:

* aws_instance.master.0: Expected 1 AMI for ID: ami-06d864b4154214132, got none

But between openshift/release#1405, openshift/release#1317, and #221, I don't see anyone asking for that AMI angmore.

/retest

@crawford
Copy link
Contributor

/retest

@crawford
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 13, 2018
sallyom
sallyom suggested changes Sep 14, 2018
View reviewed changes
hack/tf-fmt.sh
@@ -9,6 +9,6 @@ else
--env IS_CONTAINER=TRUE \
Copy link
Contributor

Choose a reason for hiding this comment

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

Above, in #L6, please update /terraform to terraform i've pushed an 11.8 terraform image using the change you made to the build script-adding terraform binary to PATH, so now you terraform fmt... no leading slash.
You can verify image is available, docker pull quay.io/coreos/terraform-alpine:v0.11.8 or you can run ./hack/tf-fmt.sh from installer dir to verify all is working well (I did, it is 👍 ).

Copy link
Member Author

Choose a reason for hiding this comment

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

Above, in #L6, please update /terraform to terraform...

Done and rebased onto master with 59af76b -> 0032348.

Copy link
Member Author

Choose a reason for hiding this comment

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

Above, in #L6, please update /terraform to terraform...

It turns out that I can't actually do that until openshift/release bumps its image to v0.11.8. I've filed openshift/release#1515 with that bump, and put a temporary fixup 856350d here. Going forward, we'll want to:

  1. Land this PR.
  2. Land Bump Terraform from 0.11.7 to 0.11.8 release#1515.
  3. Revert 856350d.

Copy link
Contributor

Choose a reason for hiding this comment

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

ack yup i missed that

@wking
Bump Terraform from 0.11.7 to 0.11.8
0032348 
0.11.8 was cut on 2018-08-15 [1,2].

Removing the leading slash is because with v0.11.8, terraform-alpine
is dropping the terraform binary into the PATH [3,4,5].

[1]: https://github.com/hashicorp/terraform/releases/tag/v0.11.8
[2]: https://releases.hashicorp.com/terraform/0.11.8/
[3]: sallyom/scripts-images#3
[4]: sallyom/scripts-images@7221114
[5]: openshift#218 (comment)
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 14, 2018
wking added a commit to wking/openshift-release that referenced this pull request Sep 14, 2018
@wking
Bump Terraform from 0.11.7 to 0.11.8
bbb02d3 
0.11.8 was cut on 2018-08-15 [1,2].

The image bump moved the terraform binary into the container's PATH
with sallyom/scripts-images@931c8ee8 (terraform-alpine: Install
terraform into $PATH, 2018-08-16, sallyom/scripts-images#3), but the
installer's tf-fmt.sh was updated to match in
openshift/installer@00323486 (Bump Terraform from 0.11.7 to 0.11.8,
2018-06-09, openshift/installer#218).  Bumping the image here depends
on using that bumped version of tf-fmt.sh.

[1]: https://github.com/hashicorp/terraform/releases/tag/v0.11.8
[2]: https://releases.hashicorp.com/terraform/0.11.8/
@wking wking mentioned this pull request Sep 14, 2018
Merged
@wking
Copy link
Member Author

wking commented Sep 14, 2018

Looks like smoke hung:

2018/09/14 17:26:51 Container cli in pod e2e-aws-smoke completed successfully
2018/09/14 20:26:47 Copying artifacts from e2e-aws-smoke into /logs/artifacts/e2e-aws-smoke
2018/09/14 20:26:47 error: unable to signal to artifacts container to terminate in pod e2e-aws-smoke, triggering deletion: could not run remote command: unable to upgrade connection: container not found ("artifacts")
2018/09/14 20:26:47 error: unable to retrieve artifacts from pod e2e-aws-smoke: could not read gzipped artifacts: unable to upgrade connection: container not found ("artifacts")
E0914 20:26:52.149169      11 event.go:200] Server rejected event '&v1.Event{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:v1.ObjectMeta{Name:".15545e13d71e1ede", GenerateName:"", Namespace:"ci-op-wls43cq2", SelfLink:"", UID:"", ResourceVersion:"", Generation:0, CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}}, DeletionTimestamp:(*v1.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Initializers:(*v1.Initializers)(nil), Finalizers:[]string(nil), ClusterName:""}, InvolvedObject:v1.ObjectReference{Kind:"", Namespace:"ci-op-wls43cq2", Name:"", UID:"", APIVersion:"", ResourceVersion:"", FieldPath:""}, Reason:"CiJobFailed", Message:"Running job pull-ci-openshift-installer-e2e-aws-smoke for PR https://github.com/openshift/installer/pull/218 in namespace ci-op-wls43cq2 from author wking", Source:v1.EventSource{Component:"ci-op-wls43cq2", Host:""}, FirstTimestamp:v1.Time{Time:time.Time{wall:0xbedf244308bda6de, ext:11061067160280, loc:(*time.Location)(0x19b8fc0)}}, LastTimestamp:v1.Time{Time:time.Time{wall:0xbedf244308bda6de, ext:11061067160280, loc:(*time.Location)(0x19b8fc0)}}, Count:1, Type:"Normal", EventTime:v1.MicroTime{Time:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}}, Series:(*v1.EventSeries)(nil), Action:"", Related:(*v1.ObjectReference)(nil), ReportingController:"", ReportingInstance:""}': 'events ".15545e13d71e1ede" is forbidden: unable to create new content in namespace ci-op-wls43cq2 because it is being terminated' (will not retry!)
2018/09/14 20:26:53 Ran for 3h4m20s

/retest

@wking
Copy link
Member Author

wking commented Sep 15, 2018

/hold cancel
/retest

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 15, 2018
@wking
Copy link
Member Author

wking commented Sep 15, 2018

All green here. @crawford, can you re-lgtm at some point?

@crawford
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 16, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit a6fbe17 into openshift:master Sep 17, 2018
@wking wking deleted the terraform-0.11.8 branch September 17, 2018 01:16
@wking wking mentioned this pull request Sep 17, 2018
Merged
wking added a commit to wking/openshift-installer that referenced this pull request Sep 17, 2018
@wking
Revert "hack/tf-fmt: Add a workaround for origin/release's hardcoding…
4e9612d 
… v0.11.7"

This reverts commit 856350d, openshift#218.

openshift/release@bbb02d36 (Bump Terraform from 0.11.7 to 0.11.8,
2018-09-14, openshift/release#1515) has landed, so we can drop the
workaround.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sallyom sallyom sallyom requested changes

@abhinavdahiya abhinavdahiya Awaiting requested review from abhinavdahiya

@staebler staebler Awaiting requested review from staebler

Assignees

@crawford crawford

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@wking @crawford @openshift-ci-robot @openshift-bot @sallyom @openshift-merge-robot