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

Added support for machine image update strategy #1807

Merged
merged 3 commits into from
Apr 18, 2024

Conversation

grolu
Copy link
Contributor

@grolu grolu commented Apr 11, 2024

  • Added support for machine image update strategy
  • Use GMultiMessage component for Kubernetes version on GNewShootDetails
  • Fixed empty tooltip on worker groups
  • Show kubernetes version update information as list

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #1793

Special notes for your reviewer:

Release note:

The Dashboard now recognizes and displays automatic update notifications according to the configured `update strategy` for machine image vendors

Use GMultiMessage component for Kubernetes version on GNewShootDetails
Fixed empty tooltip on worker groups
Show kubernetes version update information as list
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 11, 2024
@gardener-robot gardener-robot added needs/review Needs review size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Apr 11, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 11, 2024
@grolu grolu added the area/ipcei IPCEI (Important Project of Common European Interest) label Apr 11, 2024
@gardener-robot
Copy link

@holgerkoser You have pull request review open invite, please check

frontend/src/components/ShootWorkers/GWorkerGroup.vue Outdated Show resolved Hide resolved
@@ -405,14 +411,13 @@ export const useCloudProfileStore = defineStore('cloudProfile', () => {
continue
}

logger.error(`Skipped machine image ${machineImage.name} as version ${versionObj.version} is not a valid semver version and cannot be normalized`)
logger.error(`Skipped machine image ${name} as version ${versionObj.version} is not a valid semver version and cannot be normalized`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be logged as error and not inside the getter. I prosose that we remove it now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it should not be logged (as an error) inside the getter. However, if there is no place where we log that we skipped some entries, this might make it harder to troubleshoot issues where an entry is expected to be there but is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the main Problem here is that the machine image getter is not cached (because of param). That's why we get the log message multiple times. We should solve the root problem. @holgerkoser removed it in his refactoring PR afaik

Copy link
Member

@holgerkoser holgerkoser Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have moved the flattening of the machineImages into an own function and call it right after data fetching. The getter now directly returns the already flattened machine images.

  async function fetchCloudProfiles () {
    const response = await api.getCloudProfiles()
    for (const cloudProfile of response.data) {
      set(cloudProfile, 'data.machineImages', flattenMachineImages(get(cloudProfile, 'data.machineImages')))
    }
    list.value = response.data
  }
  
  function flattenMachineImages (machineImages) {
    return flatMap(machineImages, machineImage => {
      const versions = filter(machineImage.versions, ({ version, expirationDate }) => {
        if (!semver.valid(version)) {
          logger.warn(`Skipped machine image ${machineImage.name} as version ${version} is not a valid semver version`)
          return false
        }
        return true
      })
      versions.sort((a, b) => {
        return semver.rcompare(a.version, b.version)
      })

      const name = machineImage.name
      const vendorName = vendorNameFromImageName(machineImage.name)
      const vendorHint = findVendorHint(configStore.vendorHints, vendorName)

      return map(versions, ({ version, expirationDate, cri, classification, architectures }) => {
        if (isEmpty(architectures)) {
          architectures = ['amd64'] // default if not maintained
        }
        return decorateClassificationObject({
          key: name + '/' + version,
          name,
          version,
          cri,
          classification,
          expirationDate,
          vendorName,
          icon: vendorName,
          vendorHint,
          architectures,
        })
      })
    })
  }
  
  function machineImagesByCloudProfileName (cloudProfileName) {
    const cloudProfile = cloudProfileByName(cloudProfileName)
    return get(cloudProfile, 'data.machineImages')
  }

frontend/__tests__/utils/index.spec.js Outdated Show resolved Hide resolved
frontend/__tests__/utils/index.spec.js Outdated Show resolved Hide resolved
frontend/__tests__/utils/index.spec.js Outdated Show resolved Hide resolved
frontend/__tests__/utils/index.spec.js Outdated Show resolved Hide resolved
frontend/__tests__/utils/index.spec.js Outdated Show resolved Hide resolved
</div>
<div v-if="shootSupportedUpgradeAvailable">
Upgrade is available for this version
<div :class="{ 'list-style': shootSupportedPatchAvailable && shootSupportedUpgradeAvailable }">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both boolean flags are false we render an empty list item content block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this cannot happen as the whole section is hidden in tis case see line 98

frontend/src/components/NewShoot/GNewShootDetails.vue Outdated Show resolved Hide resolved
@@ -405,14 +411,13 @@ export const useCloudProfileStore = defineStore('cloudProfile', () => {
continue
}

logger.error(`Skipped machine image ${machineImage.name} as version ${versionObj.version} is not a valid semver version and cannot be normalized`)
logger.error(`Skipped machine image ${name} as version ${versionObj.version} is not a valid semver version and cannot be normalized`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it should not be logged (as an error) inside the getter. However, if there is no place where we log that we skipped some entries, this might make it harder to troubleshoot issues where an entry is expected to be there but is not.

@gardener-robot gardener-robot added the size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) label Apr 18, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 18, 2024
@gardener-robot gardener-robot removed the size/l Size of pull request is large (see gardener-robot robot/bots/size.py) label Apr 18, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 18, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 18, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 18, 2024
Copy link
Member

@holgerkoser holgerkoser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review needs/second-opinion Needs second review by someone else labels Apr 18, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 18, 2024
Copy link
Member

@petersutter petersutter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@grolu grolu merged commit 5cece4e into master Apr 18, 2024
9 checks passed
@grolu grolu deleted the enh/machine_image_update_strategy branch April 18, 2024 14:27
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Apr 18, 2024
holgerkoser added a commit that referenced this pull request Apr 24, 2024
* master: (78 commits)
  remove `pragma: whitelist secret` comments (#1836)
  Ticket title should contain project name (#1824)
  Update dependency @fontsource/roboto to v5.0.13 (#1828)
  Update dependency vite to v5.2.10 (#1827)
  Update dependency jest-date-mock to v1.0.10 (#1829)
  `Terminal`: show last error description on timeout (#1810)
  Added support for machine image update strategy (#1807)
  Update dependency vue-router to v4.3.2 (#1821)
  Update dependency eslint-plugin-vue to v9.25.0 (#1811)
  Update renovate.json5
  Update renovate.json5
  Update renovate.json5
  renovate: Ignore eslint major updates (#1819)
  Update dependency vuetify to v3.5.16 (#1818)
  Update dependency vue to v3.4.23 (#1817)
  Update dependency prom-client to v15.1.2 (#1814)
  Update dependency vite to v5.2.9 (#1815)
  Update dependency vue to v3.4.22 (#1813)
  Update dependency sass to v1.75.0 (#1809)
  Update vitest monorepo to v1.5.0 (#1808)
  ...

# Conflicts:
#	.pnp.cjs
#	frontend/__tests__/stores/cloudProfile.spec.js
#	frontend/package.json
#	frontend/src/components/GSeedConfiguration.vue
#	frontend/src/components/GShootActionRotateCredentials.vue
#	frontend/src/components/GShootListRow.vue
#	frontend/src/components/GShootListRowActions.vue
#	frontend/src/components/NewShoot/GNewShootDetails.vue
#	frontend/src/components/ShootDetails/GShootDetailsCard.vue
#	frontend/src/components/ShootVersion/GShootVersion.vue
#	frontend/src/components/ShootWorkers/GManageWorkers.vue
#	frontend/src/components/ShootWorkers/GWorkerGroup.vue
#	frontend/src/mixins/shootItem.js
#	frontend/src/store/cloudProfile/index.js
#	frontend/src/utils/index.js
#	yarn.lock

Resoved conflicts and fixed tests not yet tested
@grolu grolu mentioned this pull request Jun 4, 2024
49 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ipcei IPCEI (Important Project of Common European Interest) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gardener Dashboard needs to respect updateStrategy for image auto updates
6 participants