Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MGMT-19159: Switch kube api release image cache to map #7013

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion internal/versions/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ func NewHandler(
}

if enableKubeAPI {
releaseImagesCache := convertReleaseImagesToMap(releaseImages)
h := &kubeAPIVersionsHandler{
mustGatherVersions: mustGatherVersions,
releaseImages: releaseImages,
releaseImages: releaseImagesCache,
releaseHandler: releaseHandler,
releaseImageMirror: releaseImageMirror,
log: log,
Expand Down Expand Up @@ -281,3 +282,19 @@ func validateReleaseImage(releaseImage *models.ReleaseImage) error {
// To validate CPU architecture enum
return releaseImage.Validate(strfmt.Default)
}

func convertReleaseImagesToMap(releaseImages models.ReleaseImages) map[string]*models.ReleaseImage {
releaseImagesCache := make(map[string]*models.ReleaseImage, len(releaseImages))
for _, releaseImage := range releaseImages {
releaseImagesCache[*releaseImage.URL] = releaseImage
}
return releaseImagesCache
}

func convertMapToReleaseImages(releaseImagesMap map[string]*models.ReleaseImage) models.ReleaseImages {
var releaseImages []*models.ReleaseImage
for _, img := range releaseImagesMap {
releaseImages = append(releaseImages, img)
}
return releaseImages
}
46 changes: 24 additions & 22 deletions internal/versions/kube_api_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (

type kubeAPIVersionsHandler struct {
mustGatherVersions MustGatherVersions
releaseImages models.ReleaseImages
releaseImages map[string]*models.ReleaseImage
imagesLock sync.Mutex
sem *semaphore.Weighted
releaseHandler oc.Release
Expand Down Expand Up @@ -60,10 +60,17 @@ func (h *kubeAPIVersionsHandler) GetReleaseImage(ctx context.Context, openshiftV

// The image doesn't exist in the cache.
// Fetch all the cluster image sets, cache them, then search the cache again
if err := h.CacheAllClusterImageSets(ctx, pullSecret); err != nil {
return nil, err
}

return h.getReleaseImageFromCache(openshiftVersion, cpuArchitecture)
}

func (h *kubeAPIVersionsHandler) CacheAllClusterImageSets(ctx context.Context, pullSecret string) error {
clusterImageSets := &hivev1.ClusterImageSetList{}
if err := h.kubeClient.List(ctx, clusterImageSets); err != nil {
return nil, err
return err
}
var wg sync.WaitGroup
for _, clusterImageSet := range clusterImageSets.Items {
Expand All @@ -77,13 +84,7 @@ func (h *kubeAPIVersionsHandler) GetReleaseImage(ctx context.Context, openshiftV
wg.Done()
h.sem.Release(1)
}()
existsInCache := false
for _, releaseImage := range h.releaseImages {
if releaseImage.URL != nil && *releaseImage.URL == clusterImageSet.Spec.ReleaseImage {
existsInCache = true
break
}
}
_, existsInCache := h.releaseImages[clusterImageSet.Spec.ReleaseImage]
if !existsInCache {
_, err := h.addReleaseImage(clusterImageSet.Spec.ReleaseImage, pullSecret)
if err != nil {
Expand All @@ -93,19 +94,16 @@ func (h *kubeAPIVersionsHandler) GetReleaseImage(ctx context.Context, openshiftV
}(clusterImageSet)
}
wg.Wait()

return h.getReleaseImageFromCache(openshiftVersion, cpuArchitecture)
return nil
}

// GetReleaseImageByURL retrieves a release image based on its URL.
// The function searches through the cached release images and returns the matching image if found.
// If the image is not present in the cache, it attempts to add the image to the cache by fetching its details
// (including OpenShift version and CPU architecture) using the specified URL and 'oc' / 'skopeo' CLI tools.
func (h *kubeAPIVersionsHandler) GetReleaseImageByURL(ctx context.Context, url, pullSecret string) (*models.ReleaseImage, error) {
for _, image := range h.releaseImages {
if swag.StringValue(image.URL) == url {
return image, nil
}
if img, existsInCache := h.releaseImages[url]; existsInCache {
return img, nil
}

return h.addReleaseImage(url, pullSecret)
Expand All @@ -123,13 +121,14 @@ func (h *kubeAPIVersionsHandler) getReleaseImageFromCache(openshiftVersion, cpuA
cpuArchitecture = common.DefaultCPUArchitecture
}

releaseImages := convertMapToReleaseImages(h.releaseImages)
// Filter Release images by specified CPU architecture.
exactCPUArchReleaseImages := funk.Filter(h.releaseImages, func(releaseImage *models.ReleaseImage) bool {
exactCPUArchReleaseImages := funk.Filter(releaseImages, func(releaseImage *models.ReleaseImage) bool {
return swag.StringValue(releaseImage.CPUArchitecture) == cpuArchitecture
})

// Filter multi-arch Release images by containing the specified CPU architecture
multiArchReleaseImages := funk.Filter(h.releaseImages, func(releaseImage *models.ReleaseImage) bool {
multiArchReleaseImages := funk.Filter(releaseImages, func(releaseImage *models.ReleaseImage) bool {
return *releaseImage.CPUArchitecture == common.MultiCPUArchitecture &&
funk.Contains(releaseImage.CPUArchitectures, cpuArchitecture)
})
Expand Down Expand Up @@ -190,7 +189,8 @@ func getReleaseImageByVersion(desiredOpenshiftVersion string, releaseImages []*m
// version that can be used. This functions performs a very weak matching because RHCOS versions
// are very loosely coupled with the OpenShift versions what allows for a variety of mix&match.
func (h *kubeAPIVersionsHandler) ValidateReleaseImageForRHCOS(rhcosVersion, cpuArchitecture string) error {
return validateReleaseImageForRHCOS(h.log, rhcosVersion, cpuArchitecture, h.releaseImages)
releaseImages := convertMapToReleaseImages(h.releaseImages)
return validateReleaseImageForRHCOS(h.log, rhcosVersion, cpuArchitecture, releaseImages)
}

// addReleaseImage adds a new release image to the handler's cache based on the specified image URL and pull secret.
Expand Down Expand Up @@ -223,26 +223,28 @@ func (h *kubeAPIVersionsHandler) addReleaseImage(releaseImageUrl, pullSecret str
h.imagesLock.Lock()
defer h.imagesLock.Unlock()

releaseImages := convertMapToReleaseImages(h.releaseImages)
// Fetch ReleaseImage if exists (not using GetReleaseImage as we search for the x.y.z image only)
releaseImage := funk.Find(h.releaseImages, func(releaseImage *models.ReleaseImage) bool {
releaseImage := funk.Find(releaseImages, func(releaseImage *models.ReleaseImage) bool {
return *releaseImage.OpenshiftVersion == ocpReleaseVersion && *releaseImage.CPUArchitecture == cpuArchitecture
})
if releaseImage == nil {
// Create a new ReleaseImage
releaseImage = &models.ReleaseImage{
newReleaseImage := &models.ReleaseImage{
OpenshiftVersion: &ocpReleaseVersion,
CPUArchitecture: &cpuArchitecture,
URL: &releaseImageUrl,
Version: &ocpReleaseVersion,
CPUArchitectures: cpuArchitectures,
}

// Store in releaseImages array
h.releaseImages = append(h.releaseImages, releaseImage.(*models.ReleaseImage))
// Store in releaseImages cache
h.releaseImages[releaseImageUrl] = newReleaseImage
h.log.Infof("Stored release version %s for architecture %s", ocpReleaseVersion, cpuArchitecture)
if len(cpuArchitectures) > 1 {
h.log.Infof("Full list or architectures: %s", cpuArchitectures)
}
return newReleaseImage, nil
}

return releaseImage.(*models.ReleaseImage), nil
Expand Down
33 changes: 27 additions & 6 deletions internal/versions/kube_api_versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ var _ = Describe("GetReleaseImage", func() {
h = &kubeAPIVersionsHandler{
log: common.GetTestLog(),
releaseHandler: mockRelease,
releaseImages: defaultReleaseImages,
releaseImages: convertReleaseImagesToMap(defaultReleaseImages),
sem: semaphore.NewWeighted(30),
}
schemes := runtime.NewScheme()
Expand Down Expand Up @@ -274,7 +274,7 @@ var _ = Describe("GetReleaseImage", func() {
})

It("returns the matching CPU architecture over multi-arch if it is present", func() {
h.releaseImages = models.ReleaseImages{
h.releaseImages = convertReleaseImagesToMap(models.ReleaseImages{
&models.ReleaseImage{
CPUArchitecture: swag.String(common.MultiCPUArchitecture),
CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture, common.PowerCPUArchitecture},
Expand All @@ -289,7 +289,7 @@ var _ = Describe("GetReleaseImage", func() {
URL: swag.String("release_4.12.999-x86_64"),
Version: swag.String("4.12.999"),
},
}
})

releaseImage, err := h.GetReleaseImage(ctx, "4.12.999", common.X86CPUArchitecture, pullSecret)
Expect(err).ShouldNot(HaveOccurred())
Expand All @@ -299,7 +299,7 @@ var _ = Describe("GetReleaseImage", func() {
})

It("returns the multi-arch image if matching CPU architecture is not present", func() {
h.releaseImages = models.ReleaseImages{
h.releaseImages = convertReleaseImagesToMap(models.ReleaseImages{
&models.ReleaseImage{
CPUArchitecture: swag.String(common.MultiCPUArchitecture),
CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture, common.PowerCPUArchitecture},
Expand All @@ -314,7 +314,7 @@ var _ = Describe("GetReleaseImage", func() {
URL: swag.String("release_4.13.999-x86_64"),
Version: swag.String("4.13.999"),
},
}
})

releaseImage, err := h.GetReleaseImage(ctx, "4.12.999", common.X86CPUArchitecture, pullSecret)
Expect(err).ShouldNot(HaveOccurred())
Expand Down Expand Up @@ -397,6 +397,27 @@ var _ = Describe("GetReleaseImage", func() {
}
}
})
Context("CacheAllClusterImageSets", func() {
It("adds all release images to the cache and all requested images can be found", func() {
releaseImageURL := "example.com/openshift-release-dev/ocp-release:4.11.999"
cis := &hivev1.ClusterImageSet{
ObjectMeta: metav1.ObjectMeta{Name: "new-release"},
Spec: hivev1.ClusterImageSetSpec{ReleaseImage: releaseImageURL},
}
Expect(client.Create(ctx, cis)).To(Succeed())

mockRelease.EXPECT().GetOpenshiftVersion(gomock.Any(), releaseImageURL, "", pullSecret).Return("4.11.999", nil).Times(1)
mockRelease.EXPECT().GetReleaseArchitecture(gomock.Any(), releaseImageURL, "", pullSecret).Return([]string{common.X86CPUArchitecture}, nil).Times(1)
err := h.CacheAllClusterImageSets(ctx, pullSecret)
Expect(err).NotTo(HaveOccurred())
image, err := h.GetReleaseImage(ctx, "4.11.999", common.X86CPUArchitecture, pullSecret)
Expect(err).ToNot(HaveOccurred())
Expect(image.URL).To(HaveValue(Equal(releaseImageURL)))
image, err = h.GetReleaseImage(ctx, "4.11.999", common.X86CPUArchitecture, pullSecret)
Expect(err).ToNot(HaveOccurred())
Expect(image.URL).To(HaveValue(Equal(releaseImageURL)))
})
})
})

var _ = Describe("GetReleaseImageByURL", func() {
Expand All @@ -418,7 +439,7 @@ var _ = Describe("GetReleaseImageByURL", func() {
h = &kubeAPIVersionsHandler{
log: common.GetTestLog(),
releaseHandler: mockRelease,
releaseImages: defaultReleaseImages,
releaseImages: convertReleaseImagesToMap(defaultReleaseImages),
sem: semaphore.NewWeighted(30),
}
})
Expand Down