Skip to content

Commit

Permalink
MGMT-15070: Unable to change machine-network with dual stack (#5349)
Browse files Browse the repository at this point in the history
When working with dual stack, and there is an attempt to update the
machine networks only, they are not updated.  This change fixes it.
  • Loading branch information
ori-amizur committed Jul 11, 2023
1 parent 10a88f5 commit 61b295a
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
4 changes: 2 additions & 2 deletions internal/bminventory/inventory.go
Expand Up @@ -2112,6 +2112,7 @@ func (b *bareMetalInventory) updateNonDhcpNetworkParams(updates map[string]inter
// We need to do it because updateNonDhcpNetworkParams is called before updateNetworks, so
// inside this function here cluster object has still values before applying requested changes.
targetConfiguration := common.Cluster{}
targetConfiguration.ID = cluster.ID
targetConfiguration.ClusterNetworks = cluster.ClusterNetworks
targetConfiguration.ServiceNetworks = cluster.ServiceNetworks
targetConfiguration.MachineNetworks = cluster.MachineNetworks
Expand Down Expand Up @@ -2171,7 +2172,6 @@ func (b *bareMetalInventory) updateNonDhcpNetworkParams(updates map[string]inter
// require that user explicitly provides all the Machine Networks.
if reqDualStack {
if params.ClusterUpdateParams.MachineNetworks != nil {
cluster.MachineNetworks = params.ClusterUpdateParams.MachineNetworks
primaryMachineNetworkCidr = string(params.ClusterUpdateParams.MachineNetworks[0].Cidr)
} else {
primaryMachineNetworkCidr = network.GetPrimaryMachineCidrForUserManagedNetwork(cluster, log)
Expand All @@ -2182,7 +2182,7 @@ func (b *bareMetalInventory) updateNonDhcpNetworkParams(updates map[string]inter
log.WithError(err).Warnf("Verify dual-stack machine networks")
return common.NewApiError(http.StatusBadRequest, err)
}
secondaryMachineNetworkCidr, err = network.GetSecondaryMachineCidr(cluster)
secondaryMachineNetworkCidr, err = network.GetSecondaryMachineCidr(&targetConfiguration)
if err != nil {
return common.NewApiError(http.StatusBadRequest, err)
}
Expand Down
33 changes: 30 additions & 3 deletions internal/bminventory/inventory_test.go
Expand Up @@ -17283,11 +17283,14 @@ var _ = Describe("Dual-stack cluster", func() {
})

Context("Update cluster", func() {
var params installer.V2UpdateClusterParams
var (
params installer.V2UpdateClusterParams
clusterID strfmt.UUID
)

BeforeEach(func() {
mockUsageReports()
clusterID := strfmt.UUID(uuid.New().String())
clusterID = strfmt.UUID(uuid.New().String())
err := db.Create(&common.Cluster{Cluster: models.Cluster{
ID: &clusterID,
OpenshiftVersion: common.TestDefaultConfig.OpenShiftVersion,
Expand Down Expand Up @@ -17339,8 +17342,32 @@ var _ = Describe("Dual-stack cluster", func() {
verifyApiErrorString(reply, http.StatusBadRequest, "Expected 2 machine networks, found 1")
})
})
It("update dual machine networks", func() {
By("setup dual stack networking")
cluster := &common.Cluster{
Cluster: models.Cluster{
ID: &clusterID,
},
}
mockClusterApi.EXPECT().VerifyClusterUpdatability(createClusterIdMatcher(cluster)).Return(nil).Times(2)
params.ClusterUpdateParams.MachineNetworks = common.TestDualStackNetworking.MachineNetworks
params.ClusterUpdateParams.ServiceNetworks = common.TestDualStackNetworking.ServiceNetworks
params.ClusterUpdateParams.ClusterNetworks = common.TestDualStackNetworking.ClusterNetworks
cls, err := bm.UpdateClusterNonInteractive(ctx, params)
Expect(err).ToNot(HaveOccurred())
Expect(cls.MachineNetworks).To(HaveLen(2))
for _, m := range common.TestDualStackNetworking.MachineNetworks {
Expect(cls.MachineNetworks).To(ContainElement(&models.MachineNetwork{ClusterID: clusterID, Cidr: m.Cidr}))
}
By("update machine networks")
params.ClusterUpdateParams.MachineNetworks = append([]*models.MachineNetwork{}, common.TestDualStackNetworking.MachineNetworks...)
params.ClusterUpdateParams.MachineNetworks[1] = &models.MachineNetwork{ClusterID: clusterID, Cidr: "3001:db8::/120"}
cls, err = bm.UpdateClusterNonInteractive(ctx, params)
Expect(err).ToNot(HaveOccurred())
Expect(cls.MachineNetworks).To(HaveLen(2))
Expect(cls.MachineNetworks).To(ContainElement(&models.MachineNetwork{ClusterID: clusterID, Cidr: "3001:db8::/120"}))
})
})

})

var _ = Describe("GetCredentials", func() {
Expand Down

0 comments on commit 61b295a

Please sign in to comment.