Skip to content

Conversation

@kikisdeliveryservice
Copy link
Contributor

@kikisdeliveryservice kikisdeliveryservice commented Jun 26, 2019

Operator.go & bootstrap.go need to reference the MCO image instead of the
old setupetcdenv image.

Update template to add setupetcdenv entrypoint for MCO image.

Required-for: openshift/installer#1875
Related-to: #850
Related-to: #739 (issue will only close when final installer pr merges)

cc: @runcom

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 26, 2019
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 26, 2019
@kikisdeliveryservice kikisdeliveryservice changed the title templates: update template to replace setupetcdenv image with MCO image [WIP] templates: update template to replace setupetcdenv image with MCO image Jun 26, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 26, 2019
@runcom
Copy link
Member

runcom commented Jun 26, 2019

/approve

@kikisdeliveryservice kikisdeliveryservice changed the title [WIP] templates: update template to replace setupetcdenv image with MCO image templates: update template to replace setupetcdenv image with MCO image Jun 26, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 26, 2019
@ericavonb
Copy link

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2019
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@kikisdeliveryservice
Copy link
Contributor Author

kikisdeliveryservice commented Jun 26, 2019

@runcom Other people are seeing this failure in BZ: 1724346 other people are seeing the same: hyperkube[1339]: E0626 21:47:25.979131 1339 runtime.go:69] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference) as this pr starting early today. Will investigate more.

@openshift-bot
Copy link
Contributor

/retest

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

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2019
@kikisdeliveryservice
Copy link
Contributor Author

(Pushed a commit updating the author date to stop the bot's /retest insanity for now.)

@kikisdeliveryservice
Copy link
Contributor Author

Hitting BZ 1724346

See in tests logs e2e-aws: artifacts/e2e-aws/installer/logbundle..../bootstrap/journals/kubelet.log :
Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer

@kikisdeliveryservice
Copy link
Contributor Author

namespace deleted, trying again

/retest

@kikisdeliveryservice kikisdeliveryservice mentioned this pull request Jul 1, 2019
@runcom
Copy link
Member

runcom commented Jul 2, 2019

/retest

@runcom
Copy link
Member

runcom commented Jul 2, 2019

This 100% fails with that panic 🤔 I'm wondering if something else is going on here (but upgrade passed!)

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 2, 2019
@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 2, 2019
@kikisdeliveryservice kikisdeliveryservice force-pushed the setupetcenv-image branch 3 times, most recently from bf9f810 to abedb3d Compare July 3, 2019 00:27
@kikisdeliveryservice
Copy link
Contributor Author

kikisdeliveryservice commented Jul 3, 2019

Ok I can confirm that the problem is this branch. The mco image isn't being passed in, in the template- templates/master/00-master/_base/files/etc-kubernetes-manifests-etcd-member.yaml- (tho it is passed elsewhere successfully from the bootstrap.go/operator.go):

Jul 02 22:28:13 ip-10-0-134-0 hyperkube[1002]: I0702 22:28:13.014075 1002 kuberuntime_manager.go:617] computePodActions got {KillPod:false CreateSandbox:false SandboxID:08f5fe55cac2d4092e184e2c95436da69e0e4f14e557c5196535127ae9992919 Attempt:0 NextInitContainerToStart:&Container{Name:discovery,Image:<no value>,Command:[/usr/bin/setup-etcd-environment],Args:[run --discovery-srv=ci-op-2y2yd539-c4a31.origin-ci-int-aws.dev.rhcloud.com --output-file=/run/etcd/environment --v=4],WorkingDir:,Ports:[],Env:

Key part: "Image: < no value > ,Command:[/usr/bin/setup-etcd-environment]"

This is from the template where I have: Images.MachineConfigOperator that value isn't being passed somehow.

The above is from controlplane/1.1.somenumber/journals/kubelet.log

UPDATE: I think I got it..

initContainers:
- name: discovery
image: "{{.Images.setupEtcdEnv}}"
image: "{{.Images.machineConfigOperator}}"
Copy link
Contributor Author

@kikisdeliveryservice kikisdeliveryservice Jul 3, 2019

Choose a reason for hiding this comment

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

so there's an issue here that is still failing. I thought I found a fix by subbing MachineConfigOperator -> machineConfigOperator I did this bc I noticed the image referenced was setupEtcdEnv and kubeClientAgentImage which is from https://github.com/openshift/machine-config-operator/blob/master/install/0000_80_machine-config-operator_02_images.configmap.yaml as opposed to images.go like elsewhere. I think this change is correct, but there is something missing for this machineconfigoperator image to be seen from the templates/ and im really not sure why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this image somehow referenced before it has a value? (since this is in a different place than the other mc* images)

Operator.go & bootstrap.go need to reference the MCO image instead of the
old setupetcdenv image.

Update template to add setupetcdenv entrypoint for MCO image.

Required-for: openshift/installer#1875
Related-to: openshift#850
Related-to: openshift#739 (issue will only close when final installer pr merges)
@runcom
Copy link
Member

runcom commented Jul 3, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ericavonb, kikisdeliveryservice, runcom

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [ericavonb,kikisdeliveryservice,runcom]

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

@openshift-merge-robot openshift-merge-robot merged commit 253ed18 into openshift:master Jul 3, 2019
runcom added a commit to runcom/machine-config-operator that referenced this pull request Jul 4, 2019
Honestly finding images in the struct confusing as it's really the
collection of two different sets of images (renderConfig and
ControllerConfig).
This also led to some hassles around changing images in openshift#899 for
instance.
This patch adds some comments around the scope of the images, split the
set in two different ones (still tied to the original struct) and change
the template keys used to substite.
@kikisdeliveryservice kikisdeliveryservice deleted the setupetcenv-image branch July 10, 2019 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

6 participants