Changes required for 4.1.0-rc3 #523

stbenjam · 2019-05-09T17:11:01Z

Changes required for update to RC.3

This PR
Add api-int record to bootstrap kni-installer#77
Libvirt api-int record: Add VIP DNS entries to vm_setup_vars.yml #536, vm-setup: remove additional DNS records metal3-io/metal3-dev-env#9
Handle api-int case for baremetal: TBD
Merge openshift/installer for 4.1.0-rc.3 kni-installer#75
Resolve machine-api failure machine-api is failing on 4.1.0 rc.3 #537
Change name of machine labels Change name of Machine labels. kni-installer#80

russellb

I wonder if this is still the right registry, even for a public release. Do you know?

russellb · 2019-05-09T17:59:05Z

looks like it should be on quay.io/openshift-release-dev/ocp-release

stbenjam · 2019-05-09T18:16:42Z

It does work with this url, but seems to end up redirecting to quay in the end

russellb

Switching will drop the need for the extra CI registry pull secret

intlabs · 2019-05-09T22:20:10Z

Switching will drop the need for the extra CI registry pull secret

Confirmed:
$ docker pull quay.io/openshift-release-dev/ocp-release:4.1.0-rc.1
4.1.0-rc.1: Pulling from openshift-release-dev/ocp-release
5dfdb3f0bcc0: Pull complete
99f178453a43: Pull complete
f9fa6d5c56a6: Pull complete
d252d93af3e8: Pull complete
a9607047a865: Pull complete
Digest: sha256:be61a9ec132118e41a417b347242361d9cc96b1a73753e121dc7a74a1905baea
Status: Downloaded newer image for quay.io/openshift-release-dev/ocp-release:4.1.0-rc.1

russellb · 2019-05-13T02:16:06Z

We should also look at #401 - if that's ready, would be good to base our next update on this new approach

stbenjam · 2019-05-13T10:15:37Z

I’ve been watching the work on that. I agree, would like to use that process. Looks like a few things left to do on the checklist at the top of the PR before it’s ready.

assets/templates/99_registry.yaml

stbenjam · 2019-05-13T19:47:09Z

vm-setup/roles/common/tasks/main.yml

                - ip: "{{ baremetal_network_cidr | nthhost(5) }}"
                  hostnames:
                    - "api"
+                    - "api-int"


This change is still needed, at least for now - the masters are pointing at dnsmasq when they come up and they get their ignition config from api-int

I guess this is fine as an interim solution in the VM case, but how do we avoid requiring a new DNS record for the API VIP in the baremetal case? cc @celebdor and @cybertron

The solution is to get the VIPs from install config and serve them in coredns (and make ignition fetch from VIP instead of api)

openshift-metal3/kni-installer#77 makes api-int available from coredns. We'll still need the change to point ignition at the VIP instead of a DNS name though.

If I'm looking in the right place, doesn't ignition happen using HTTPS, verifying the hostname and certificate? The endpoint is also not platform specific: how do we make that work?

https://github.com/openshift/installer/blob/master/pkg/asset/ignition/machine/node.go#L23-L25

We also need to tweak the certificate so it includes the VIP in addition to the DNS name. Some work has been done in that direction by other teams and I need to look at getting it integrated into our fork of the installer.

utils.sh

stbenjam · 2019-05-13T20:13:30Z

I was able to get a successful 4.1.0 rc.3 install, using the PR's
listed below. I didn't do any deep testing but my cluster looks
functional.

hardys · 2019-05-15T10:01:14Z

Ah sorry this will need a rebase since vm-setup moved to metal3-dev-env - I wonder if we should parameterize some of vm-setup/roles/common/tasks/main.yml so that we can control the DNS things which are specific to OpenShift via the vm_setup_vars.yml in dev-scripts?

ocp_install_env.sh

markmc · 2019-05-15T11:09:17Z

@stbenjam want to add a checklist at the top listing what needs to be done before we can merge? Finding it hard to follow ...

hardys · 2019-05-15T11:26:51Z

@stbenjam want to add a checklist at the top listing what needs to be done before we can merge? Finding it hard to follow ...

Sounds good, but I think the main remaining item is to resolve the api-int DNS solution, what's proposed here will only work for VMs AFAICS and we want to avoid requiring those testing baremetal to create another upstream DNS record for the "internal" VIP.

So we need to figure out how ignition can resolve the IP in both VM and baremetal cases, which as mentioned by @stbenjam is tricky as I don't think we can just replace the hostname with an IP, and the resolv.conf additions don't happen until after the ignition/pivot happens, so we have a chicken/egg problem?

stbenjam · 2019-05-15T11:26:54Z

I updated the description with the checklist. I'll rebase this PR when I'm at my workstation. Essentially,
nothing's changed since #523 (comment) - but there are a number of open and WIP PR's related to the api-int change. It's not clear to me if @cybertron @celebdor want to get something else in, or if they are ok with only the 3 listed above which worked for me.

markmc · 2019-05-15T11:30:13Z

@stbenjam want to add a checklist at the top listing what needs to be done before we can merge? Finding it hard to follow ...

Sounds good, but I think the main remaining item is to resolve the api-int DNS solution, what's proposed here will only work for VMs AFAICS and we want to avoid requiring those testing baremetal to create another upstream DNS record for the "internal" VIP.

So we need to figure out how ignition can resolve the IP in both VM and baremetal cases, which as mentioned by @stbenjam is tricky as I don't think we can just replace the hostname with an IP, and the resolv.conf additions don't happen until after the ignition/pivot happens, so we have a chicken/egg problem?

Thanks for the explanation.

I suggest we proceed with rebasing ASAP and add "remove the need for an api-int DNS entry on baremetal" to our backlog?

stbenjam · 2019-05-15T12:12:02Z

@hardys I rebased, and moved the api-int change here: metal3-io/metal3-dev-env#8

That's a bit specific to openshift, isn't it? Maybe we should parametrize those records in that role?

hardys · 2019-05-15T12:17:52Z

@hardys I rebased, and moved the api-int change here: metal3-io/metal3-dev-env#8

That's a bit specific to openshift, isn't it? Maybe we should parametrize those records in that role?

Thanks! And yes, as mentioned in my earlier comment we probably should parameterize/sanitize some of that but I missed it in my earlier refactoring - I can take a look unless you've already started something?

hardys · 2019-05-15T13:45:03Z

I just pushed #536 which would enable us to add api-int via a variable only to dev-scripts, it will need to be merged together with metal3-io/metal3-dev-env#9

- update structure of cluster-image-registry-operator config object to match openshift/release#3548 - host-etcd moved from kube-system namespace to openshift-etcd

markmc · 2019-05-16T06:15:28Z

We should be able to proceed with registry.svc.ci.openshift.org/kni/release:4.1.0-rc.3-kni.0 as a temporary workaround for #537 - see https://gist.github.com/markmc/f8a78d7cea7252a9e0f29dadcfaa1253 for how it was built

stbenjam · 2019-05-16T13:36:18Z

Updated the pin, and I'm testing a new deployment right now. Thanks!

stbenjam · 2019-05-16T14:55:02Z

It comes up fine, but hitting an issue on worker deployments. Based on discussion in slack, it sounds like it's pre-existing on 4.0 as well. The worker node shows "Node not found hook failed: Port 00:f6:ff:4d:bb:31 already exists" when it boots

russellb · 2019-05-16T16:28:48Z

It comes up fine, but hitting an issue on worker deployments. Based on discussion in slack, it sounds like it's pre-existing on 4.0 as well. The worker node shows "Node not found hook failed: Port 00:f6:ff:4d:bb:31 already exists" when it boots

This isn't the real error. You see this because we haven't actually provisioned the worker host at all, and the baremetal-operator powers on hosts that aren't provisioned if we have online: true set on them. It's questionable whether that behavior makes sense, but it's behaving as expected.

The real error is up in the cluster-api integration that prevented the worker from being provisioned. See #537 (comment)

russellb reviewed May 9, 2019

View reviewed changes

stbenjam mentioned this pull request May 10, 2019

Merge openshift/installer for 4.1.0-rc.3 openshift-metal3/kni-installer#75

Merged

stbenjam force-pushed the rc-pin branch from c9d5baf to ce548c8 Compare May 13, 2019 16:23

stbenjam changed the title ~~Pin release image to 4.1.0-rc1~~ Pin release image to 4.1.0-rc3 May 13, 2019

stbenjam changed the title ~~Pin release image to 4.1.0-rc3~~ Changes required for 4.1.0-rc3 May 13, 2019

stbenjam commented May 13, 2019

View reviewed changes

assets/templates/99_registry.yaml Show resolved Hide resolved

stbenjam commented May 13, 2019

View reviewed changes

utils.sh Show resolved Hide resolved

stbenjam mentioned this pull request May 13, 2019

Add api-int record to bootstrap openshift-metal3/kni-installer#77

Merged

markmc suggested changes May 15, 2019

View reviewed changes

ocp_install_env.sh Outdated Show resolved Hide resolved

stbenjam force-pushed the rc-pin branch from a443afb to c6056f6 Compare May 15, 2019 12:09

hardys mentioned this pull request May 15, 2019

Add record for api-int to libvirt dnsmasq metal3-io/metal3-dev-env#8

Closed

stbenjam force-pushed the rc-pin branch 3 times, most recently from 613fdf9 to 176d9fd Compare May 15, 2019 13:43

stbenjam added 2 commits May 15, 2019 09:47

Pin release image to 4.1.0-rc3

122ad84

Additional fixes for RC.3

0de1207

- update structure of cluster-image-registry-operator config object to match openshift/release#3548 - host-etcd moved from kube-system namespace to openshift-etcd

stbenjam force-pushed the rc-pin branch from 176d9fd to 0de1207 Compare May 15, 2019 13:47

This was referenced May 15, 2019

api-int should not be a required external DNS record #538

Closed

Add api-int DNS entry to dnsmasq #462

Closed

Pin to custom KNI release

5782dcc

russellb merged commit 0bca6a6 into openshift-metal3:master May 16, 2019

Changes required for 4.1.0-rc3 #523

Changes required for 4.1.0-rc3 #523

Uh oh!

Conversation

stbenjam commented May 9, 2019 • edited by russellb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

russellb left a comment

Choose a reason for hiding this comment

Uh oh!

russellb commented May 9, 2019

Uh oh!

stbenjam commented May 9, 2019

Uh oh!

russellb left a comment

Choose a reason for hiding this comment

Uh oh!

intlabs commented May 9, 2019

Uh oh!

russellb commented May 13, 2019

Uh oh!

stbenjam commented May 13, 2019

Uh oh!

Uh oh!

stbenjam May 13, 2019

Choose a reason for hiding this comment

Uh oh!

hardys May 14, 2019

Choose a reason for hiding this comment

Uh oh!

celebdor May 14, 2019

Choose a reason for hiding this comment

Uh oh!

cybertron May 14, 2019

Choose a reason for hiding this comment

Uh oh!

stbenjam May 14, 2019

Choose a reason for hiding this comment

Uh oh!

cybertron May 15, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stbenjam commented May 13, 2019

Uh oh!

hardys commented May 15, 2019

Uh oh!

Uh oh!

markmc commented May 15, 2019

Uh oh!

hardys commented May 15, 2019

Uh oh!

stbenjam commented May 15, 2019

Uh oh!

markmc commented May 15, 2019

Uh oh!

stbenjam commented May 15, 2019

Uh oh!

hardys commented May 15, 2019

Uh oh!

hardys commented May 15, 2019

Uh oh!

markmc commented May 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stbenjam commented May 16, 2019

Uh oh!

stbenjam commented May 16, 2019

Uh oh!

russellb commented May 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

stbenjam commented May 9, 2019 •

edited by russellb

Loading

markmc commented May 16, 2019 •

edited

Loading