assisted-service icon indicating copy to clipboard operation
assisted-service copied to clipboard
verified publisher icon openshift

MGMT-19001, MGMT-19005: Add dhcp and autoconf fields if missing in nmstate YAML + fix the nmstate flow bug on archs other than x86

Open linoyaslan opened this issue 1 year ago • 10 comments

  1. Due to this <RHEL-59935> bug in the nmstate service, we have encountered a regression. In the previous flow, we used GC, which set autoconf and dhcp to false by default if they were not specified in the YAML. Now, with the nmstate service, it relies on the current state, which may default to true, causing an issue. I’ve implemented a temporary workaround to add these fields with false values if the user does not specify them in the YAML.

  2. Fix the nmstate flow bug on architectures other than x86.

List all the issues related to this PR

  • [ ] New Feature
  • [ ] Enhancement
  • [X] Bug fix
  • [ ] Tests
  • [ ] Documentation
  • [ ] CI/CD

What environments does this code impact?

  • [ ] Automation (CI, tools, etc)
  • [ ] Cloud
  • [ ] Operator Managed Deployments
  • [x] None

How was this code tested?

  • [X] assisted-test-infra environment
  • [ ] dev-scripts environment
  • [ ] Reviewer's test appreciated
  • [ ] Waiting for CI to do a full test run
  • [ ] Manual (Elaborate on how it was tested)
  • [ ] No tests needed

Checklist

  • [X] Title and description added to both, commit and PR.
  • [X] Relevant issues have been associated (see CONTRIBUTING guide)
  • [X] This change does not require a documentation update (docstring, docs, README, etc)
  • [x] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

  • Are the title and description (in both PR and commit) meaningful and clear?
  • Is there a bug required (and linked) for this change?
  • Should this PR be backported?

linoyaslan avatar Sep 24 '24 15:09 linoyaslan

@linoyaslan: This pull request references MGMT-19001 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.

This pull request references MGMT-19005 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.

In response to this:

  1. Due to this <RHEL-59935> bug in the nmstate service, we have encountered a regression. In the previous flow, we used GC, which set autoconf and dhcp to false by default if they were not specified in the YAML. Now, with the nmstate service, it relies on the current state, which may default to true, causing an issue. I’ve implemented a temporary workaround to add these fields with false values if the user does not specify them in the YAML.

  2. Fix the nmstate flow bug on architectures other than x86.

List all the issues related to this PR

  • [ ] New Feature
  • [ ] Enhancement
  • [X] Bug fix
  • [ ] Tests
  • [ ] Documentation
  • [ ] CI/CD

What environments does this code impact?

  • [ ] Automation (CI, tools, etc)
  • [ ] Cloud
  • [ ] Operator Managed Deployments
  • [x] None

How was this code tested?

  • [X] assisted-test-infra environment
  • [ ] dev-scripts environment
  • [ ] Reviewer's test appreciated
  • [ ] Waiting for CI to do a full test run
  • [ ] Manual (Elaborate on how it was tested)
  • [ ] No tests needed

Checklist

  • [X] Title and description added to both, commit and PR.
  • [X] Relevant issues have been associated (see CONTRIBUTING guide)
  • [X] This change does not require a documentation update (docstring, docs, README, etc)
  • [ ] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

  • Are the title and description (in both PR and commit) meaningful and clear?
  • Is there a bug required (and linked) for this change?
  • Should this PR be backported?

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot avatar Sep 24 '24 15:09 openshift-ci-robot

/cc @AlonaKaplan @eifrach @danielerez

linoyaslan avatar Sep 24 '24 15:09 linoyaslan

Codecov Report

Attention: Patch coverage is 46.66667% with 16 lines in your changes missing coverage. Please review.

Project coverage is 68.87%. Comparing base (0296c50) to head (39bfde3). Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
pkg/staticnetworkconfig/generator.go 48.14% 10 Missing and 4 partials :warning:
internal/bminventory/inventory.go 0.00% 2 Missing :warning:
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6798      +/-   ##
==========================================
+ Coverage   68.74%   68.87%   +0.13%     
==========================================
  Files         249      249              
  Lines       37254    37561     +307     
==========================================
+ Hits        25611    25872     +261     
- Misses       9365     9392      +27     
- Partials     2278     2297      +19
Flag Coverage Δ
68.87% <46.66%> (+0.13%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/ignition/discovery.go 73.80% <100.00%> (ø)
internal/bminventory/inventory.go 70.72% <0.00%> (ø)
pkg/staticnetworkconfig/generator.go 68.75% <48.14%> (-1.76%) :arrow_down:

... and 3 files with indirect coverage changes

codecov[bot] avatar Sep 24 '24 16:09 codecov[bot]

/test ?

linoyaslan avatar Sep 24 '24 18:09 linoyaslan

@linoyaslan: The following commands are available to trigger required jobs:

  • /test e2e-agent-compact-ipv4
  • /test edge-assisted-operator-catalog-publish-verify
  • /test edge-ci-index
  • /test edge-e2e-ai-operator-ztp
  • /test edge-e2e-ai-operator-ztp-sno-day2-workers
  • /test edge-e2e-ai-operator-ztp-sno-day2-workers-late-binding
  • /test edge-e2e-metal-assisted
  • /test edge-e2e-metal-assisted-4-12
  • /test edge-e2e-metal-assisted-4-control-planes-4-17
  • /test edge-e2e-metal-assisted-4-control-planes-4-18
  • /test edge-e2e-metal-assisted-5-control-planes-4-17
  • /test edge-e2e-metal-assisted-5-control-planes-4-18
  • /test edge-e2e-metal-assisted-cnv-4-16
  • /test edge-e2e-metal-assisted-lvm
  • /test edge-e2e-metal-assisted-odf-4-16
  • /test edge-images
  • /test edge-lint
  • /test edge-subsystem-aws
  • /test edge-subsystem-kubeapi-aws
  • /test edge-unit-test
  • /test edge-verify-generated-code
  • /test images
  • /test mce-images

The following commands are available to trigger optional jobs:

  • /test e2e-agent-ha-dualstack
  • /test e2e-agent-sno-ipv6
  • /test edge-e2e-ai-operator-disconnected-capi
  • /test edge-e2e-ai-operator-ztp-3masters
  • /test edge-e2e-ai-operator-ztp-capi
  • /test edge-e2e-ai-operator-ztp-compact-day2-masters
  • /test edge-e2e-ai-operator-ztp-compact-day2-workers
  • /test edge-e2e-ai-operator-ztp-disconnected
  • /test edge-e2e-ai-operator-ztp-hypershift-zero-nodes
  • /test edge-e2e-ai-operator-ztp-multiarch-3masters-ocp
  • /test edge-e2e-ai-operator-ztp-multiarch-sno-ocp
  • /test edge-e2e-ai-operator-ztp-node-labels
  • /test edge-e2e-ai-operator-ztp-remove-node
  • /test edge-e2e-ai-operator-ztp-sno-day2-masters
  • /test edge-e2e-ai-operator-ztp-sno-day2-workers-ignitionoverride
  • /test edge-e2e-metal-assisted-4-13
  • /test edge-e2e-metal-assisted-4-14
  • /test edge-e2e-metal-assisted-4-15
  • /test edge-e2e-metal-assisted-4-16
  • /test edge-e2e-metal-assisted-bond
  • /test edge-e2e-metal-assisted-bond-4-12
  • /test edge-e2e-metal-assisted-bond-4-13
  • /test edge-e2e-metal-assisted-bond-4-14
  • /test edge-e2e-metal-assisted-bond-4-15
  • /test edge-e2e-metal-assisted-bond-4-16
  • /test edge-e2e-metal-assisted-day2
  • /test edge-e2e-metal-assisted-day2-arm-workers
  • /test edge-e2e-metal-assisted-day2-single-node
  • /test edge-e2e-metal-assisted-external
  • /test edge-e2e-metal-assisted-external-4-14
  • /test edge-e2e-metal-assisted-ipv4v6
  • /test edge-e2e-metal-assisted-ipv6
  • /test edge-e2e-metal-assisted-kube-api-late-binding-single-node
  • /test edge-e2e-metal-assisted-kube-api-late-unbinding-ipv4-single-node
  • /test edge-e2e-metal-assisted-kube-api-net-suite
  • /test edge-e2e-metal-assisted-mce-4-16
  • /test edge-e2e-metal-assisted-mce-sno-4-16
  • /test edge-e2e-metal-assisted-metallb
  • /test edge-e2e-metal-assisted-none
  • /test edge-e2e-metal-assisted-onprem
  • /test edge-e2e-metal-assisted-single-node
  • /test edge-e2e-metal-assisted-static-ip-suite
  • /test edge-e2e-metal-assisted-static-ip-suite-4-12
  • /test edge-e2e-metal-assisted-static-ip-suite-4-13
  • /test edge-e2e-metal-assisted-static-ip-suite-4-14
  • /test edge-e2e-metal-assisted-static-ip-suite-4-15
  • /test edge-e2e-metal-assisted-static-ip-suite-4-16
  • /test edge-e2e-metal-assisted-tang
  • /test edge-e2e-metal-assisted-tpmv2
  • /test edge-e2e-metal-assisted-upgrade-agent
  • /test edge-e2e-nutanix-assisted
  • /test edge-e2e-nutanix-assisted-2workers
  • /test edge-e2e-nutanix-assisted-4-14
  • /test edge-e2e-oci-assisted
  • /test edge-e2e-oci-assisted-4-14
  • /test edge-e2e-oci-assisted-iscsi
  • /test edge-e2e-vsphere-assisted
  • /test edge-e2e-vsphere-assisted-4-14
  • /test edge-e2e-vsphere-assisted-4-15
  • /test edge-e2e-vsphere-assisted-4-16
  • /test edge-e2e-vsphere-assisted-umn
  • /test okd-scos-images
  • /test push-pr-image

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openshift-assisted-service-master-e2e-agent-compact-ipv4
  • pull-ci-openshift-assisted-service-master-edge-ci-index
  • pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp
  • pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted
  • pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted-4-control-planes-4-17
  • pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted-4-control-planes-4-18
  • pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted-5-control-planes-4-17
  • pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted-5-control-planes-4-18
  • pull-ci-openshift-assisted-service-master-edge-images
  • pull-ci-openshift-assisted-service-master-edge-lint
  • pull-ci-openshift-assisted-service-master-edge-subsystem-aws
  • pull-ci-openshift-assisted-service-master-edge-subsystem-kubeapi-aws
  • pull-ci-openshift-assisted-service-master-edge-unit-test
  • pull-ci-openshift-assisted-service-master-edge-verify-generated-code
  • pull-ci-openshift-assisted-service-master-images
  • pull-ci-openshift-assisted-service-master-mce-images

In response to this:

/test ?

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-sigs/prow repository.

openshift-ci[bot] avatar Sep 24 '24 18:09 openshift-ci[bot]

/test edge-e2e-metal-assisted-bond /test edge-e2e-metal-assisted-static-ip-suite

linoyaslan avatar Sep 24 '24 18:09 linoyaslan

@linoyaslan: This pull request references MGMT-19001 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.

This pull request references MGMT-19005 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.

In response to this:

  1. Due to this <RHEL-59935> bug in the nmstate service, we have encountered a regression. In the previous flow, we used GC, which set autoconf and dhcp to false by default if they were not specified in the YAML. Now, with the nmstate service, it relies on the current state, which may default to true, causing an issue. I’ve implemented a temporary workaround to add these fields with false values if the user does not specify them in the YAML.

  2. Fix the nmstate flow bug on architectures other than x86.

List all the issues related to this PR

  • [ ] New Feature
  • [ ] Enhancement
  • [X] Bug fix
  • [ ] Tests
  • [ ] Documentation
  • [ ] CI/CD

What environments does this code impact?

  • [ ] Automation (CI, tools, etc)
  • [ ] Cloud
  • [ ] Operator Managed Deployments
  • [x] None

How was this code tested?

  • [X] assisted-test-infra environment
  • [ ] dev-scripts environment
  • [ ] Reviewer's test appreciated
  • [ ] Waiting for CI to do a full test run
  • [ ] Manual (Elaborate on how it was tested)
  • [ ] No tests needed

Checklist

  • [X] Title and description added to both, commit and PR.
  • [X] Relevant issues have been associated (see CONTRIBUTING guide)
  • [X] This change does not require a documentation update (docstring, docs, README, etc)
  • [x] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

  • Are the title and description (in both PR and commit) meaningful and clear?
  • Is there a bug required (and linked) for this change?
  • Should this PR be backported?

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot avatar Sep 24 '24 20:09 openshift-ci-robot

/lgtm (retest failed jobs, seems unrelated)

danielerez avatar Sep 24 '24 21:09 danielerez

/override ci/prow/edge-e2e-metal-assisted-4-control-planes-4-17 ci/prow/edge-e2e-metal-assisted-5-control-planes-4-17 ci/prow/edge-e2e-metal-assisted-4-control-planes-4-18 ci/prow/edge-e2e-metal-assisted-5-control-planes-4-18

danmanor avatar Sep 25 '24 10:09 danmanor

@danmanor: Overrode contexts on behalf of danmanor: ci/prow/edge-e2e-metal-assisted-4-control-planes-4-17, ci/prow/edge-e2e-metal-assisted-4-control-planes-4-18, ci/prow/edge-e2e-metal-assisted-5-control-planes-4-17, ci/prow/edge-e2e-metal-assisted-5-control-planes-4-18

In response to this:

/override ci/prow/edge-e2e-metal-assisted-4-control-planes-4-17 ci/prow/edge-e2e-metal-assisted-5-control-planes-4-17 ci/prow/edge-e2e-metal-assisted-4-control-planes-4-18 ci/prow/edge-e2e-metal-assisted-5-control-planes-4-18

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-sigs/prow repository.

openshift-ci[bot] avatar Sep 25 '24 10:09 openshift-ci[bot]

/test edge-e2e-metal-assisted-bond /test edge-e2e-metal-assisted-static-ip-suite

linoyaslan avatar Sep 25 '24 16:09 linoyaslan

/test e2e-agent-sno-ipv6 /test edge-e2e-ai-operator-disconnected-capi /test edge-e2e-metal-assisted-ipv4v6 /test edge-e2e-ai-operator-ztp-disconnected

linoyaslan avatar Sep 25 '24 16:09 linoyaslan

/approve /lgtm

danielerez avatar Sep 25 '24 17:09 danielerez

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danielerez, linoyaslan

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:
  • ~~OWNERS~~ [danielerez,linoyaslan]

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

openshift-ci[bot] avatar Sep 25 '24 17:09 openshift-ci[bot]

/retest

linoyaslan avatar Sep 25 '24 21:09 linoyaslan

@linoyaslan: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agent-sno-ipv6 39bfde38fcddfb327fce98d56e518165ba5af5db link false /test e2e-agent-sno-ipv6

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-sigs/prow repository. I understand the commands that are listed here.

openshift-ci[bot] avatar Sep 26 '24 01:09 openshift-ci[bot]

[ART PR BUILD NOTIFIER]

Distgit: ose-agent-installer-api-server This PR has been included in build ose-agent-installer-api-server-container-v4.18.0-202409260309.p0.gc3b2f89.assembly.stream.el9. All builds following this will include this PR.

openshift-bot avatar Sep 26 '24 03:09 openshift-bot