Bug 2115770: Check for RendezvousIP in Agent config if NMStateConfig is not provided #6236

lranjbar · 2022-08-17T13:35:21Z

Digging around I found this function already implemented so I decided to use it.

andfasano · 2022-08-17T15:05:20Z

/lgtm

openshift-ci · 2022-08-17T15:10:42Z

@lranjbar: This pull request references Bugzilla bug 2115770, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.12.0) matches configured target release for branch (4.12.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @zniu1011

Details

In response to this:

Bug 2115770: Check for RendezvousIP in Agent config if NMStateConfig is not provided

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.

pkg/agent/rest.go

zaneb

/approve

pkg/agent/rest.go

openshift-ci · 2022-08-17T18:23:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zaneb

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:

~~pkg/agent/OWNERS~~ [zaneb]
~~pkg/asset/agent/OWNERS~~ [zaneb]

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

lranjbar · 2022-08-17T21:21:53Z

/test ?

openshift-ci · 2022-08-17T21:21:56Z

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

/test e2e-agent-compact
/test gofmt
/test golint
/test govet
/test images
/test unit
/test verify-codegen
/test verify-vendor

The following commands are available to trigger optional jobs:

/test e2e-aws

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

pull-ci-openshift-installer-agent-installer-e2e-agent-compact
pull-ci-openshift-installer-agent-installer-gofmt
pull-ci-openshift-installer-agent-installer-golint
pull-ci-openshift-installer-agent-installer-govet
pull-ci-openshift-installer-agent-installer-images
pull-ci-openshift-installer-agent-installer-unit
pull-ci-openshift-installer-agent-installer-verify-codegen
pull-ci-openshift-installer-agent-installer-verify-vendor

Details

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/test-infra repository.

lranjbar · 2022-08-17T21:22:09Z

/test verify-vendor

zaneb

Looks good, just some error handling to tidy up.

pkg/agent/rest.go

andfasano · 2022-08-18T14:41:14Z

pkg/agent/rest.go

Just as a reminder note (not for this PR), I think in future it could be useful (at least for the maintenance) to deploy a (non-writable) RendezvousIP asset (with a dependency on AgentConfig and AgentManifests - if not directly NMStateConfig), to be used directly here and in the Ignition asset

I like this idea. After going through this exercise I think we can consolidate the logic for the RendezvousIP into it's own asset that will clean it up a lot for the future.

…s not provided

bfournie · 2022-08-18T22:16:26Z

/lgtm

openshift-ci · 2022-08-18T23:13:53Z

@lranjbar: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

openshift-ci · 2022-08-18T23:20:08Z

@lranjbar: All pull requests linked via external trackers have merged:

openshift/installer#6236

Bugzilla bug 2115770 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2115770: Check for RendezvousIP in Agent config if NMStateConfig is not provided

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-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2022

openshift-ci bot requested review from celebdor and pawanpinjarkar August 17, 2022 13:36

lranjbar force-pushed the bz-2115770 branch from cb9e8be to 44ea6ff Compare August 17, 2022 13:44

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2022

lranjbar force-pushed the bz-2115770 branch from 44ea6ff to a1cadf9 Compare August 17, 2022 14:27

openshift-ci bot assigned andfasano Aug 17, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2022

zaneb changed the title ~~BZ-2115770: Check for RendezvousIP in Agent config if NMStateConfig is not provided~~ Bug 2115770: Check for RendezvousIP in Agent config if NMStateConfig is not provided Aug 17, 2022

openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Aug 17, 2022

openshift-ci bot requested a review from zniu1011 August 17, 2022 15:10

pawanpinjarkar reviewed Aug 17, 2022

View reviewed changes

pkg/agent/rest.go Outdated Show resolved Hide resolved

zaneb reviewed Aug 17, 2022

View reviewed changes

pkg/agent/rest.go Outdated Show resolved Hide resolved

lranjbar force-pushed the bz-2115770 branch from a1cadf9 to 3e0804b Compare August 17, 2022 15:44

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2022

lranjbar force-pushed the bz-2115770 branch from 3e0804b to 63c6e93 Compare August 17, 2022 16:09

zaneb reviewed Aug 17, 2022

View reviewed changes

pkg/agent/rest.go Outdated Show resolved Hide resolved

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2022

lranjbar force-pushed the bz-2115770 branch from 1e7f93b to 24be98d Compare August 17, 2022 22:05

zaneb reviewed Aug 18, 2022

View reviewed changes

pkg/agent/rest.go Outdated Show resolved Hide resolved

pkg/agent/rest.go Outdated Show resolved Hide resolved

pkg/agent/rest.go Outdated Show resolved Hide resolved

andfasano reviewed Aug 18, 2022

View reviewed changes

lranjbar force-pushed the bz-2115770 branch 2 times, most recently from a51e9a2 to 88785b4 Compare August 18, 2022 19:30

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2022

lranjbar added 2 commits August 18, 2022 14:44

BZ-2115770: Check for RendezvousIP in Agent config if NMStateConfig i…

2daf365

…s not provided

BZ-2115770: Use assetStore.Load() instead of assetStore.Fetch()

9dde79a

lranjbar force-pushed the bz-2115770 branch from 88785b4 to 9dde79a Compare August 18, 2022 21:52

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2022

openshift-ci bot assigned bfournie Aug 18, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2022

openshift-merge-robot merged commit 404766c into openshift:agent-installer Aug 18, 2022

Bug 2115770: Check for RendezvousIP in Agent config if NMStateConfig is not provided #6236

Bug 2115770: Check for RendezvousIP in Agent config if NMStateConfig is not provided #6236

Conversation

lranjbar commented Aug 17, 2022

Uh oh!

andfasano commented Aug 17, 2022

Uh oh!

openshift-ci bot commented Aug 17, 2022

Uh oh!

Uh oh!

Uh oh!

zaneb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-ci bot commented Aug 17, 2022

Uh oh!

lranjbar commented Aug 17, 2022

Uh oh!

openshift-ci bot commented Aug 17, 2022

Uh oh!

lranjbar commented Aug 17, 2022

Uh oh!

zaneb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andfasano Aug 18, 2022

Choose a reason for hiding this comment

Uh oh!

lranjbar Aug 18, 2022

Choose a reason for hiding this comment

Uh oh!

bfournie commented Aug 18, 2022

Uh oh!

openshift-ci bot commented Aug 18, 2022

Uh oh!

openshift-ci bot commented Aug 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants