Skip to content

Conversation

@vrutkovs
Copy link
Contributor

Cherry-picks of master commits since last rebase

yboaronn and others added 20 commits April 13, 2020 10:36
This PR updates HAProxy static pod manifest to enable reading API LB backend details
from Kubernetes instead of _etcd-server-ssl._tcp SRV.
We have a serious bug in how we backup "original" files and restore them.
Here, "original" means files that ship with RHCOS. Think of a default Chrony
or another system daemon configuration file. When the MCD kicks in and writes
to those files, we want to backup the original one (the shipped-with-RHCOS) in order
to restore it if a user deletes the MC that modified it (this was the initial bug reported
in GitHub at openshift#782).

However, that patch that fixed openshift#782
was causing the following; if you shipped a file with just _one_ MC, removing it would wipe it out and that works.
However, if you modified that file later again with another MC, a backup
file will be created for the first MC, and when deleting the file by deleting the second MC, it will
restore the initial file shipped with the first MC instead of wiping it out completely which
it should have since that file was never meant to be backed up because it wasn't on RHCOS from the beginning.

This patch now differentiates between files that are already on RHCOS (on-disk so to speak)
and files that are shipped with an MC. For the former, the MCD will create a backup as it's doing today,
for the latter instead, the MCD creates a placeholder file that tells it to just get rid
of the file altogether (along with adding all the necessary checks and actions in order to create those backup files).

The issue popped up on upgrade paths where the new manifests rendered by the MCO don't contain a certain file.
The MCD notices that and go ahead trying to remove the file. It however notices that a backup file
(which was created for an MC shipped file and later other MC have modified it) is there and tries to restore it (also failing
with invalid cross-link device error, but that's another issue which I'm fixing here as well by using cp directly).

Really hoping all the above makes sense.

Signed-off-by: Antonio Murdaca <runcom@linux.com>
Avoids querying the network and download vendors in the CI mainly.

Signed-off-by: Antonio Murdaca <runcom@linux.com>
This commit adds two new files:
- A systemd unit that runs a shell script
- A shell script that runs vmtoolsd to retrieve `guestinfo.hostname`
from vm guest. Then uses hostnamectl to set hostname.
The MCO runs a bunch of sync functions during its sync. Each of these functions have
a name (a task name). When we loop through the sync functions and fail on any of them,
the MCO breaks and reports the Degraded status for that task as Reason: TaskNameFailed.
It then goes and retry all the sync functions (no change in behavior till here).
During some debugging and testing I've noticed that the RenderConfig task failed with the
version mismatch error for osImageURL. That is good, and also right. The MCO went ahead
and set the Degraded condition to RenderConfigFailed (correctly!). The MCO then went again
and re-tried all the sync functions. Now the RenderConfig task succeeded (the version mismatch error
can indeed be temporary). What happened from now on is that the MCO kept rolling and eventually finishes
the upgrade I was running but the RenderConfigFailed Degraded status was never cleared back to False
even if the task _did_ succeed.
This patch introduces a new function which we call after every sync function in order to clear
any previous degraded status when it instaed succeeded on later syncs.

Signed-off-by: Antonio Murdaca <runcom@linux.com>
Add a check for backwards compatibility: basically if the file
doesn't exist in /usr/etc/ and no rpm is claming it, we assume
that the orig file came from a wrongful backup of a MachineConfig
file instead of a RHCOS file. In this case we delete the `orig`
file and don't back up.

Also change the name to noOrigFileStamp for clarity.

Signed-off-by: Yu Qi Zhang <jerzhang@redhat.com>
This patch adds the workaround suggested on [1]
to make nodeport work, instead of ethtool we use
NM to apply the fix for each connction before it is up.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1794714

Signed-off-by: Gal Zaidman <gzaidman@redhat.com>
Signed-off-by: Antonio Murdaca <runcom@linux.com>
as it's difficult to debug with cri-o only set to log level error

Signed-off-by: Peter Hunt <pehunt@redhat.com>
When keepalived sets the VIP, it triggers a connection event in
NetworkManager that, since the name we set from DHCP was transient it
would do a reverse lookup on the connection address. Unfortunately, NM
does not filter out the deprecated VIP for the reverse lookup and ends
up overriding the hostname with DNS names that map to VIPs configured in
the system.

This fix is a workaround that on environments that have DHCP provide the
hostname, will prevent the erroneous behavior of NM by setting DHCP
provided FQDN addresses as static, which prevents NM from doing further
address lookups.

On hostname-less DHCP environments we'd want to hook to the hostname
event in NM and make sure that the first one that does not map to a VIP
is the one that gets set as static.

Signed-off-by: Antoni Segura Puimedon <antoni@redhat.com>
On PR [1] we added a workaround for Bug [2], this
fails when the worker starts for the first time
since openshift-sdn is created only when the
sdn pod is starting.
Instead we will disable by default leave as is
only when running with OVNkubernetes

[1] On PR openshift#1606,
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1794714

Signed-off-by: Gal Zaidman <gzaidman@redhat.com>
This patch adds the workaround suggested on [1]
to make nodeport work, instead of ethtool we use
NM to apply the fix for each connction before it is up.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1794714

Brings the following ovirt fixes to openstack platform:
- openshift#1606
- openshift#1621
This is inferred by the golang-1.13 container now

Remove now erroring copying of scripts and add workaround for
k8s.io/code-generator not playing nice with go modules.
Restore code-generator's go.mod file and
remove vendor/k8s.io/code-generator/vendor directory
after running verification in`make verify`
A map doesn't guarantee order when we are creating new ignitions.
When we update the image CR with blocked registries, the ctrcfg
controller needs to update two files registries.conf and policy.json.
Since we get an update from the image CR about every 20 mins, we compare
the semantics to see if anything has changed. But since the order is not
guaranteed, the controller might think that the semantics is not equal
even if nothing in the data changed. Hence another MC is created, and
everytime we get an update the MC applied to the nodes keeps flipping
back and forth for the 2 possible orders causing the nodes to reboot a
bunch of times. So move to using a struct array to ensure the order is
always the same and we don't have two similar MCs being created.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
@vrutkovs
Copy link
Contributor Author

/cc @LorbusChris

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 13, 2020
@vrutkovs
Copy link
Contributor Author

/uncc mtrmac umohnani8

@LorbusChris
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 14, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LorbusChris, vrutkovs

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 [LorbusChris,vrutkovs]

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 1de18b1 into openshift:fcos Apr 14, 2020
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.