From 2daf36501964196c181ed35b870bc0a9bc4cebd2 Mon Sep 17 00:00:00 2001 From: Lisa Rashidi-Ranjbar Date: Wed, 17 Aug 2022 06:29:49 -0700 Subject: [PATCH 1/2] BZ-2115770: Check for RendezvousIP in Agent config if NMStateConfig is not provided --- pkg/agent/rest.go | 21 +++++++++++++++------ pkg/asset/agent/image/ignition.go | 5 +++-- pkg/asset/agent/image/ignition_test.go | 8 ++++---- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/pkg/agent/rest.go b/pkg/agent/rest.go index 2e54ce05b64..b41ae9856b8 100644 --- a/pkg/agent/rest.go +++ b/pkg/agent/rest.go @@ -14,6 +14,8 @@ import ( "github.com/openshift/assisted-service/client/installer" "github.com/openshift/assisted-service/models" + "github.com/openshift/installer/pkg/asset/agent/agentconfig" + "github.com/openshift/installer/pkg/asset/agent/image" "github.com/openshift/installer/pkg/asset/agent/manifests" assetstore "github.com/openshift/installer/pkg/asset/store" ) @@ -28,25 +30,32 @@ type NodeZeroRestClient struct { // NewNodeZeroRestClient Initialize a new rest client to interact with the Agent Rest API on node zero. func NewNodeZeroRestClient(ctx context.Context, assetDir string) (*NodeZeroRestClient, error) { + restClient := &NodeZeroRestClient{} + agentManifests := &manifests.AgentManifests{} + agentConfigAsset := &agentconfig.AgentConfig{} assetStore, err := assetstore.NewStore(assetDir) if err != nil { return nil, errors.Wrap(err, "failed to create asset store") } - nmState := &manifests.NMStateConfig{} - if err := assetStore.Fetch(nmState); err != nil { - return nil, errors.Wrapf(err, "failed to fetch %s", nmState.Name()) + + if agentConfigError := assetStore.Fetch(agentConfigAsset); agentConfigError != nil { + logrus.Debug(errors.Wrapf(agentConfigError, "failed to fetch %s", agentConfigAsset.Name())) + } else if manifestError := assetStore.Fetch(agentManifests); manifestError != nil { + logrus.Debug(errors.Wrapf(manifestError, "failed to fetch %s", agentManifests.Name())) + return nil, errors.New("failed to fetch AgentConfig or NMStateConfig") } - NodeZeroIP, err := manifests.GetNodeZeroIP(nmState.Config) + RendezvousIP, err := image.RetrieveRendezvousIP(agentConfigAsset.Config, agentManifests.NMStateConfigs) if err != nil { return nil, err } + config := client.Config{} config.URL = &url.URL{ Scheme: "http", - Host: net.JoinHostPort(NodeZeroIP, "8090"), + Host: net.JoinHostPort(RendezvousIP, "8090"), Path: client.DefaultBasePath, } client := client.New(config) @@ -54,7 +63,7 @@ func NewNodeZeroRestClient(ctx context.Context, assetDir string) (*NodeZeroRestC restClient.Client = client restClient.ctx = ctx restClient.config = config - restClient.NodeZeroIP = NodeZeroIP + restClient.NodeZeroIP = RendezvousIP return restClient, nil } diff --git a/pkg/asset/agent/image/ignition.go b/pkg/asset/agent/image/ignition.go index b3a843dfd94..5d167337448 100644 --- a/pkg/asset/agent/image/ignition.go +++ b/pkg/asset/agent/image/ignition.go @@ -124,7 +124,7 @@ func (a *Ignition) Generate(dependencies asset.Parents) error { return errors.New("at least one NMState configuration must be provided") } - nodeZeroIP, err := retrieveRendezvousIP(agentConfigAsset.Config, agentManifests.NMStateConfigs) + nodeZeroIP, err := RetrieveRendezvousIP(agentConfigAsset.Config, agentManifests.NMStateConfigs) if err != nil { return err } @@ -370,7 +370,8 @@ func addExtraManifests(config *igntypes.Config, extraManifests *manifests.ExtraM } } -func retrieveRendezvousIP(agentConfig *agent.Config, nmStateConfigs []*v1beta1.NMStateConfig) (string, error) { +// RetrieveRendezvousIP Returns the Rendezvous IP from either AgentConfig or NMStateConfig +func RetrieveRendezvousIP(agentConfig *agent.Config, nmStateConfigs []*v1beta1.NMStateConfig) (string, error) { var err error var rendezvousIP string diff --git a/pkg/asset/agent/image/ignition_test.go b/pkg/asset/agent/image/ignition_test.go index 651bafa08a0..b66f3ff0447 100644 --- a/pkg/asset/agent/image/ignition_test.go +++ b/pkg/asset/agent/image/ignition_test.go @@ -148,9 +148,9 @@ func TestIgnition_addStaticNetworkConfig(t *testing.T) { } func TestRetrieveRendezvousIP(t *testing.T) { - rawConfig := `interfaces: - - ipv4: - address: + rawConfig := `interfaces: + - ipv4: + address: - ip: "192.168.122.21"` cases := []struct { Name string @@ -204,7 +204,7 @@ func TestRetrieveRendezvousIP(t *testing.T) { } for _, tc := range cases { t.Run(tc.Name, func(t *testing.T) { - rendezvousIP, err := retrieveRendezvousIP(tc.agentConfig, tc.nmStateConfigs) + rendezvousIP, err := RetrieveRendezvousIP(tc.agentConfig, tc.nmStateConfigs) if tc.expectedError != "" { assert.Regexp(t, tc.expectedError, err.Error()) } else { From 9dde79a0b7e88cbd41ccc083ddb4886e8c147efa Mon Sep 17 00:00:00 2001 From: Lisa Rashidi-Ranjbar Date: Wed, 17 Aug 2022 13:31:40 -0700 Subject: [PATCH 2/2] BZ-2115770: Use assetStore.Load() instead of assetStore.Fetch() --- pkg/agent/rest.go | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/pkg/agent/rest.go b/pkg/agent/rest.go index b41ae9856b8..715a0e78a81 100644 --- a/pkg/agent/rest.go +++ b/pkg/agent/rest.go @@ -9,6 +9,7 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" + "github.com/openshift/assisted-service/api/v1beta1" "github.com/openshift/assisted-service/client" "github.com/openshift/assisted-service/client/events" "github.com/openshift/assisted-service/client/installer" @@ -18,6 +19,7 @@ import ( "github.com/openshift/installer/pkg/asset/agent/image" "github.com/openshift/installer/pkg/asset/agent/manifests" assetstore "github.com/openshift/installer/pkg/asset/store" + "github.com/openshift/installer/pkg/types/agent" ) // NodeZeroRestClient is a struct to interact with the Agent Rest API that is on node zero. @@ -30,26 +32,43 @@ type NodeZeroRestClient struct { // NewNodeZeroRestClient Initialize a new rest client to interact with the Agent Rest API on node zero. func NewNodeZeroRestClient(ctx context.Context, assetDir string) (*NodeZeroRestClient, error) { - restClient := &NodeZeroRestClient{} - agentManifests := &manifests.AgentManifests{} agentConfigAsset := &agentconfig.AgentConfig{} + agentManifestsAsset := &manifests.AgentManifests{} assetStore, err := assetstore.NewStore(assetDir) if err != nil { return nil, errors.Wrap(err, "failed to create asset store") } - if agentConfigError := assetStore.Fetch(agentConfigAsset); agentConfigError != nil { - logrus.Debug(errors.Wrapf(agentConfigError, "failed to fetch %s", agentConfigAsset.Name())) - } else if manifestError := assetStore.Fetch(agentManifests); manifestError != nil { - logrus.Debug(errors.Wrapf(manifestError, "failed to fetch %s", agentManifests.Name())) - return nil, errors.New("failed to fetch AgentConfig or NMStateConfig") + agentConfig, agentConfigError := assetStore.Load(agentConfigAsset) + agentManifests, manifestError := assetStore.Load(agentManifestsAsset) + + if agentConfigError != nil { + logrus.Debug(errors.Wrapf(agentConfigError, "failed to load %s", agentConfigAsset.Name())) + } + if manifestError != nil { + logrus.Debug(errors.Wrapf(manifestError, "failed to load %s", agentManifestsAsset.Name())) + } + if agentConfigError != nil || manifestError != nil { + return nil, errors.New("failed to load AgentConfig or NMStateConfig") } - RendezvousIP, err := image.RetrieveRendezvousIP(agentConfigAsset.Config, agentManifests.NMStateConfigs) - if err != nil { - return nil, err + var RendezvousIP string + var rendezvousIPError error + var emptyNMStateConfigs []*v1beta1.NMStateConfig + + if agentConfig != nil && agentManifests != nil { + RendezvousIP, rendezvousIPError = image.RetrieveRendezvousIP(agentConfig.(*agentconfig.AgentConfig).Config, agentManifests.(*manifests.AgentManifests).NMStateConfigs) + } else if agentConfig == nil && agentManifests != nil { + RendezvousIP, rendezvousIPError = image.RetrieveRendezvousIP(&agent.Config{}, agentManifests.(*manifests.AgentManifests).NMStateConfigs) + } else if agentConfig != nil && agentManifests == nil { + RendezvousIP, rendezvousIPError = image.RetrieveRendezvousIP(agentConfig.(*agentconfig.AgentConfig).Config, emptyNMStateConfigs) + } else { + return nil, errors.New("both AgentConfig and NMStateConfig are empty") + } + if rendezvousIPError != nil { + return nil, rendezvousIPError } config := client.Config{}