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.

Sign up for GitHub

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

Jump to bottom

Add sku version logging #2412

Merged
merged 1 commit into from
Jun 23, 2022
Merged

Add sku version logging #2412

merged 1 commit into from
Jun 23, 2022

Conversation

mboersma
Copy link
Contributor

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Adds some logging that mentions the VM image SKU version, which is now important since it encodes the Kubernetes version for reference images. This also shortens the function name and does some minor cleanup.

Which issue(s) this PR fixes:

Refs #2388 (comment)

Special notes for your reviewer:

The output looks like this (newlines added by me to highlight version):

controller/azuremachine/controllers.AzureMachineReconciler.reconcileNormal "msg"="Reconciling AzureMachine" "name"="default-18717-control-plane-x58s2" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureMachine" "x-ms-correlation-request-id"="6fc79888-c169-4e58-bf10-0eb75963e5a0" 
I0622 21:54:44.587121      38 machine.go:644] controller/azuremachine/scope.MachineScope.GetVMImage "msg"="No image specified for machine, using default Linux Image" "name"="default-18717-control-plane-x58s2" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureMachine" "x-ms-correlation-request-id"="6fc79888-c169-4e58-bf10-0eb75963e5a0" "machine"="default-18717-control-plane-x58s2"
I0622 21:54:44.587997      38 images.go:124] controller/azuremachine/virtualmachineimages.Service.getSKUAndVersion "msg"="Getting VM image SKU and version" "name"="default-18717-control-plane-x58s2" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureMachine" "x-ms-correlation-request-id"="6fc79888-c169-4e58-bf10-0eb75963e5a0" "k8sVersion"="v1.24.1" "location"="eastus" "offer"="capi" "osAndVersion"="ubuntu-2004" "publisher"="cncf-upstream"
I0622 21:54:44.588751      38 cache.go:117] controller/azuremachine/virtualmachineimages.Cache.Get "msg"="VM images cache miss" "name"="default-18717-control-plane-x58s2" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureMachine" "x-ms-correlation-request-id"="6fc79888-c169-4e58-bf10-0eb75963e5a0" "location"="eastus" "offer"="capi" "publisher"="cncf-upstream" "sku"="ubuntu-2004-gen1"
I0622 21:54:45.743926      38 images.go:176] controller/azuremachine/virtualmachineimages.Service.getSKUAndVersion "msg"="Found VM image SKU and version" "name"="default-18717-control-plane-x58s2" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureMachine" "x-ms-correlation-request-id"="6fc79888-c169-4e58-bf10-0eb75963e5a0" "location"="eastus" "offer"="capi" "publisher"="cncf-upstream" 
    "sku"="ubuntu-2004-gen1" "version"="124.1.20220601"
I0622 21:55:17.789607      38 images.go:124] controller/azuremachine/virtualmachineimages.Service.getSKUAndVersion "msg"="Getting VM image SKU and version" "name"="default-18717-control-plane-x58s2" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureMachine" "x-ms-correlation-request-id"="a83b742c-72a0-4933-9bc5-7cd9cb545a0d" "k8sVersion"="v1.24.1" "location"="eastus" "offer"="capi" "osAndVersion"="ubuntu-2004" "publisher"="cncf-upstream"
I0622 21:55:17.791411      38 cache.go:122] controller/azuremachine/virtualmachineimages.Cache.Get "msg"="VM images cache hit" "name"="default-18717-control-plane-x58s2" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureMachine" "x-ms-correlation-request-id"="a83b742c-72a0-4933-9bc5-7cd9cb545a0d" "location"="eastus" "offer"="capi" "publisher"="cncf-upstream" "sku"="ubuntu-2004-gen1"
I0622 21:55:17.791842      38 images.go:176] controller/azuremachine/virtualmachineimages.Service.getSKUAndVersion "msg"="Found VM image SKU and version" "name"="default-18717-control-plane-x58s2" "namespace"="default" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="AzureMachine" "x-ms-correlation-request-id"="a83b742c-72a0-4933-9bc5-7cd9cb545a0d" "location"="eastus" "offer"="capi" "publisher"="cncf-upstream" 
    "sku"="ubuntu-2004-gen1" "version"="124.1.20220601"

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 22, 2022
@mboersma mboersma mentioned this pull request Jun 22, 2022
Merged
3 tasks
CecileRobertMichon
CecileRobertMichon reviewed Jun 23, 2022
View reviewed changes
azure/services/virtualmachineimages/images.go
@@ -115,12 +115,14 @@ func (s *Service) GetDefaultWindowsImage(ctx context.Context, location, k8sVersi
return defaultImage, nil
}

// GetDefaultImageSKUID gets the SKU ID and version of the image to use for the provided version of Kubernetes.
// getSKUAndVersion gets the SKU ID and version of the image to use for the provided version of Kubernetes.
Copy link
Contributor

Choose a reason for hiding this comment

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

is this one meant to merge before or after #2388 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either way should be fine, they're independent (although rebases may happen). I'd like to see #2388 merge soon because it fixes a serious bug, but this just improves logging.

@jsturtevant
Copy link
Contributor

Thanks! this will help with debugging
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 23, 2022
@CecileRobertMichon
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 23, 2022
@mboersma
Copy link
Contributor Author

/retest

error: the server doesn't have a resource type "nodes"

I don't think this ci-entrypoint failure is related to this PR. I'll look into it.

@k8s-ci-robot k8s-ci-robot merged commit a68b0d2 into kubernetes-sigs:main Jun 23, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.4 milestone Jun 23, 2022
@mboersma mboersma deleted the vmimages-cleanup branch August 8, 2022 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CecileRobertMichon CecileRobertMichon CecileRobertMichon left review comments

@jackfrancis jackfrancis Awaiting requested review from jackfrancis

@Jont828 Jont828 Awaiting requested review from Jont828

Assignees

@jsturtevant jsturtevant

@CecileRobertMichon CecileRobertMichon

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.
4 participants
@mboersma @jsturtevant @CecileRobertMichon @k8s-ci-robot