OCPBUGS-16227: make sure sshKey are not emptied out on firstboot #3829
Conversation
openshift-ci-robot
added
jira/severity-critical
|
@cdoern: This pull request references Jira Issue OCPBUGS-16227, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
jira/invalid-bug
openshift-ci
bot
added
the
approved
|
/retest-required |
|
/retest-required |
|
/payload-job periodic-ci-openshift-hypershift-release-4.14-periodics-e2e-aws-ovn-conformance |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@cdoern: This pull request references Jira Issue OCPBUGS-16227, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@cdoern: trigger 1 job(s) for the /payload-(job|aggregate) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/b0ecb100-2fd7-11ee-8bb3-b9cd3cc736bb-0 |
|
/jira refresh |
|
@cdoern: This pull request references Jira Issue OCPBUGS-16227, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@cdoern: This pull request references Jira Issue OCPBUGS-16227, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
re-implement fix for sshKeys without adding them to ignition passwd Signed-off-by: Charlie Doern <cdoern@redhat.com>
|
/retest-required |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdoern, yuqi-zhang 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 |
| cs := framework.NewClientSet("") | ||
| outNodeYoungest := bytes.NewBuffer([]byte{}) | ||
| // get nodes by newest | ||
| outNodeYoungest = runCmd(t, "oc get nodes --sort-by .metadata.creationTimestamp | tail -n 1") |
There was a problem hiding this comment.
suggestion (non-blocking): You can do these node queries via the Clientset without having to shell out and parse oc output:
package main
import (
"context"
"fmt"
"sort"
"github.com/openshift/machine-config-operator/test/framework"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
func printNodeStatusLine(node *corev1.Node) {
fmt.Printf("Node %s was created at %s. Is ready? %v\n", node.Name, node.CreationTimestamp, isNodeReady(node))
}
// This will determine whether or not a node is ready. There are additional conditions one can look at and consider, though this should be enough for this particular use-case. See https://github.com/openshift/machine-config-operator/blob/94fc0354f154e2daa923dd96defdf86448b2b327/pkg/controller/node/status.go#L237-L263 for more info.
func isNodeReady(node *corev1.Node) bool {
for _, condition := range node.Status.Conditions {
if condition.Reason == "KubeletReady" && condition.Type == corev1.NodeReady && condition.Status == corev1.ConditionTrue {
return true
}
}
return false
}
func getYoungestNodeFromPool(nodeList *corev1.NodeList) *corev1.Node {
// Sort the nodes in descending order by the creation timestamp. The youngest node will be in position '0'.
sort.Slice(nodeList.Items, func(i, j int) bool {
return nodeList.Items[i].CreationTimestamp.Time.Unix() > nodeList.Items[j].CreationTimestamp.Time.Unix()
})
youngestNode := nodeList.Items[0]
return &youngestNode
}
func main() {
cs := framework.NewClientSet("")
nodeList, err := cs.CoreV1Interface.Nodes().List(context.TODO(), metav1.ListOptions{
LabelSelector: "node-role.kubernetes.io/worker=",
})
if err != nil {
panic(err)
}
youngestNode := getYoungestNodeFromPool(nodeList)
fmt.Printf("Youngest node: ")
printNodeStatusLine(youngestNode)
fmt.Println("Nodes in worker pool:")
for _, node := range nodeList.Items {
printNodeStatusLine(&node)
}
}This will produce output like this:
Youngest node: Node ip-10-0-30-153.ec2.internal was created at 2023-08-03 08:23:03 -0400 EDT. Is ready? true
Nodes in worker pool:
Node ip-10-0-30-153.ec2.internal was created at 2023-08-03 08:23:03 -0400 EDT. Is ready? true
Node ip-10-0-2-150.ec2.internal was created at 2023-08-03 08:20:39 -0400 EDT. Is ready? true
Node ip-10-0-45-21.ec2.internal was created at 2023-08-03 08:20:31 -0400 EDT. Is ready? trueThere was a problem hiding this comment.
Since my code sample above deals with timestamps, I'd like to bring attention to the fact that the Golang Time types in the stdlib are not marshalable. That means if you convert it to JSON and try to convert it back, you'll get weird conversion errors; if it even converts at all!
With that in mind, you may be wondering how Kubernetes handles this problem, given that we have timestamps. Here's the answer: https://github.com/kubernetes/apimachinery/blob/5cb236977966cfc829e3b94b57fa69b44e0a95bc/pkg/apis/meta/v1/time.go
|
@cdoern: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest-required unknown statuses?!!?! |
openshift-merge-robot
merged commit a41f5af
into
openshift:master
|
@cdoern: Jira Issue OCPBUGS-16227: All pull requests linked via external trackers have merged:
Jira Issue OCPBUGS-16227 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
re-implement fix for sshKeys without adding them to ignition passwd
I want to run tests on this to see what breaks and also run the hypershift conformance tests, just so we don't burn ourselves twice!
sshKeys should not be removed from nodes upon firstboot.