Skip to content

Commit

Permalink
Use the standard lib sync.Once to read json once
Browse files Browse the repository at this point in the history
Instead of manually use a boolean, use the standard library sync.Once to
only read the upgradePatches.json file once.

Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
  • Loading branch information
nunnatsa committed Jan 5, 2025
1 parent 0c579a6 commit 2d705aa
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 29 deletions.
2 changes: 1 addition & 1 deletion cmd/hyperconverged-cluster-operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func main() {
})
cmdHelper.ExitOnError(err, "Failed to set the status of the Upgradeable Operator Condition")

if err = upgradepatch.ValidateUpgradePatches(logger); err != nil {
if err = upgradepatch.Init(logger); err != nil {
eventEmitter.EmitEvent(nil, corev1.EventTypeWarning, "InitError", "Failed validating upgrade patches file")
cmdHelper.ExitOnError(err, "Failed validating upgrade patches file")
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/hyperconverged/hyperconverged_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestHyperconverged(t *testing.T) {
wd, _ := os.Getwd()
destFile = path.Join(wd, "upgradePatches.json")
Expect(commontestutils.CopyFile(destFile, path.Join(testFilesLocation, "upgradePatches.json"))).To(Succeed())
Expect(upgradepatch.ValidateUpgradePatches(GinkgoLogr)).To(Succeed())
Expect(upgradepatch.Init(GinkgoLogr)).To(Succeed())
})

AfterSuite(func() {
Expand Down
35 changes: 20 additions & 15 deletions pkg/upgradepatch/upgradePatches.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@ import (
_ "embed"
"encoding/json"
"errors"
"github.com/kubevirt/hyperconverged-cluster-operator/api/v1beta1"
"os"
"strings"
"sync"

"github.com/blang/semver/v4"
jsonpatch "github.com/evanphx/json-patch/v5"
"github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"

"github.com/kubevirt/hyperconverged-cluster-operator/api/v1beta1"
)

//go:generate go run ../../tools/crwriter/ --format=json --out=./hc.cr.json
Expand Down Expand Up @@ -159,8 +161,9 @@ func (up UpgradePatches) applyUpgradePatch(logger logr.Logger, hc *v1beta1.Hyper
}

var (
hcoUpgradeChanges UpgradePatches
hcoUpgradeChangesRead = false
hcoUpgradeChanges UpgradePatches
once = &sync.Once{}
onceErr error
)

func ApplyUpgradePatch(logger logr.Logger, hc *v1beta1.HyperConverged, knownHcoSV semver.Version) (*v1beta1.HyperConverged, error) {
Expand All @@ -176,9 +179,6 @@ var getUpgradeChangesFileLocation = func() string {
}

func readUpgradePatchesFromFile(logger logr.Logger) error {
if hcoUpgradeChangesRead {
return nil
}
hcoUpgradeChanges = UpgradePatches{}
fileLocation := getUpgradeChangesFileLocation()

Expand All @@ -188,29 +188,34 @@ func readUpgradePatchesFromFile(logger logr.Logger) error {
return err
}

defer file.Close()

jDec := json.NewDecoder(file)
err = jDec.Decode(&hcoUpgradeChanges)
if err != nil {
return err
}

hcoUpgradeChangesRead = true
return nil
}

func ValidateUpgradePatches(logger logr.Logger) error {
err := readUpgradePatchesFromFile(logger)
if err != nil {
return err
func Init(logger logr.Logger) error {
once.Do(func() {
onceErr = readUpgradePatchesFromFile(logger)
})

if onceErr != nil {
return onceErr
}

for _, p := range hcoUpgradeChanges.HCOCRPatchList {
if verr := validateUpgradePatch(p); verr != nil {
return verr
if err := validateUpgradePatch(p); err != nil {
return err
}
}
for _, r := range hcoUpgradeChanges.ObjectsToBeRemoved {
if verr := validateUpgradeLeftover(r); verr != nil {
return verr
if err := validateUpgradeLeftover(r); err != nil {
return err
}
}
return nil
Expand Down
29 changes: 17 additions & 12 deletions pkg/upgradepatch/upgradePatches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"path"
"slices"
"strings"
"sync"
"testing"

"github.com/blang/semver/v4"
Expand All @@ -27,41 +28,45 @@ var (
hcCRBytesOrig []byte
)

func resetOnce() {
once = &sync.Once{}
}

var _ = Describe("upgradePatches", func() {

BeforeEach(func() {
wd, _ := os.Getwd()
origFile = path.Join(wd, "upgradePatches.json")
Expect(commontestutils.CopyFile(origFile+".orig", origFile)).To(Succeed())
hcoUpgradeChangesRead = false
resetOnce()
hcCRBytes = slices.Clone(hcCRBytesOrig)
})

AfterEach(func() {
Expect(os.Remove(origFile + ".orig")).To(Succeed())
hcoUpgradeChangesRead = false
resetOnce()
})

Context("readUpgradeChangesFromFile", func() {
AfterEach(func() {
Expect(commontestutils.CopyFile(origFile, origFile+".orig")).To(Succeed())
hcoUpgradeChangesRead = false
Expect(ValidateUpgradePatches(GinkgoLogr)).To(Succeed())
resetOnce()
Expect(Init(GinkgoLogr)).To(Succeed())
})

It("should correctly parse and validate actual upgradePatches.json", func() {
Expect(ValidateUpgradePatches(GinkgoLogr)).To(Succeed())
Expect(Init(GinkgoLogr)).To(Succeed())
})

It("should correctly parse and validate empty upgradePatches", func() {
Expect(copyTestFile("empty.json")).To(Succeed())
Expect(ValidateUpgradePatches(GinkgoLogr)).To(Succeed())
Expect(Init(GinkgoLogr)).To(Succeed())
})

It("should fail parsing upgradePatches with bad json", func() {
Expect(copyTestFile("badJson.json")).To(Succeed())

err := ValidateUpgradePatches(GinkgoLogr)
err := Init(GinkgoLogr)
Expect(err).To(MatchError(HavePrefix("invalid character")))
})

Expand All @@ -70,15 +75,15 @@ var _ = Describe("upgradePatches", func() {
It("should fail validating upgradePatches with bad semver ranges", func() {
Expect(copyTestFile("badSemverRange.json")).To(Succeed())

err := ValidateUpgradePatches(GinkgoLogr)
err := Init(GinkgoLogr)
Expect(err).To(MatchError(HavePrefix("Could not get version from string:")))
})

DescribeTable(
"should fail validating upgradePatches with bad patches",
func(filename, message string) {
Expect(copyTestFile(filename)).To(Succeed())
Expect(ValidateUpgradePatches(GinkgoLogr)).To(MatchError(HavePrefix(message)))
Expect(Init(GinkgoLogr)).To(MatchError(HavePrefix(message)))
},
Entry(
"bad operation kind",
Expand All @@ -102,7 +107,7 @@ var _ = Describe("upgradePatches", func() {
func(filename string, expectedErr bool, message string) {
Expect(copyTestFile(filename)).To(Succeed())

err := ValidateUpgradePatches(GinkgoLogr)
err := Init(GinkgoLogr)
if expectedErr {
Expect(err).To(MatchError(HavePrefix(message)))
} else {
Expand Down Expand Up @@ -141,14 +146,14 @@ var _ = Describe("upgradePatches", func() {

It("should fail validating upgradePatches with bad semver ranges", func() {
Expect(copyTestFile("badSemverRangeOR.json")).To(Succeed())
Expect(ValidateUpgradePatches(GinkgoLogr)).To(MatchError(HavePrefix("Could not get version from string:")))
Expect(Init(GinkgoLogr)).To(MatchError(HavePrefix("Could not get version from string:")))
})

DescribeTable(
"should fail validating upgradePatches with bad patches",
func(filename, message string) {
Expect(copyTestFile(filename)).To(Succeed())
Expect(ValidateUpgradePatches(GinkgoLogr)).To(MatchError(HavePrefix(message)))
Expect(Init(GinkgoLogr)).To(MatchError(HavePrefix(message)))
},
Entry(
"empty object kind",
Expand Down

0 comments on commit 2d705aa

Please sign in to comment.