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

MachinePool: GCP: Custom UserTags #2463

Conversation

2uasimojo
Copy link
Member

@2uasimojo 2uasimojo commented Sep 19, 2024

Add a new API field, MachinePool.Spec.GCP.UserTags, and plumb it into the GCP MachineSet generator. Results end up in the GCP's providerSpec.ResourceManagerTags (per upstream installer code at the time of this commit).

NOTE: This commit breaks ground on a new way of enhancing hive's APIs based on the installer: We are including installer as a dependency in the apis/ submodule for the first time to pull in the UserTag type by reference rather than copying it as we have done in the past. The dependency tooling seems smart enough to avoid pulling all of installer's transitive dependencies into the apis/ submodule. We must be vigilant that this remains true going forward.

HIVE-2512

(cherry picked from commit c758151)

Add a new API field, MachinePool.Spec.GCP.UserTags, and plumb it into
the GCP MachineSet generator. Results end up in the GCP's
providerSpec.ResourceManagerTags (per upstream installer code at the
time of this commit).

NOTE: This commit breaks ground on a new way of enhancing hive's APIs
based on the installer: We are including installer as a dependency in
the `apis/` submodule for the first time to pull in the `UserTag` type
by reference rather than copying it as we have done in the past. The
dependency tooling seems smart enough to avoid pulling all of
installer's transitive dependencies into the `apis/` submodule. We must
be vigilant that this remains true going forward.

HIVE-2512

(cherry picked from commit c758151)
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 19, 2024
@2uasimojo
Copy link
Member Author

/assign @jstuever

Had to do this manually to get around conflicts with #2453 (which looks like it won't be backportable).

Copy link
Contributor

openshift-ci bot commented Sep 20, 2024

@2uasimojo: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Copy link

codecov bot commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 37.50000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 58.61%. Comparing base (0e0e20c) to head (d8ebe58).
Report is 2 commits behind head on mce-2.6.

Files with missing lines Patch % Lines
pkg/controller/machinepool/gcpactuator.go 37.50% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           mce-2.6    #2463      +/-   ##
===========================================
- Coverage    58.61%   58.61%   -0.01%     
===========================================
  Files          181      181              
  Lines        25820    25823       +3     
===========================================
+ Hits         15134    15135       +1     
- Misses        9413     9415       +2     
  Partials      1273     1273              
Files with missing lines Coverage Δ
pkg/controller/machinepool/gcpactuator.go 71.02% <37.50%> (-0.41%) ⬇️

@jstuever
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2024
Copy link
Contributor

openshift-ci bot commented Sep 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, jstuever

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

@openshift-merge-bot openshift-merge-bot bot merged commit 38082a1 into openshift:mce-2.6 Sep 23, 2024
10 checks passed
@2uasimojo 2uasimojo deleted the HIVE-2512/mce-2.6/gcp-usertags branch September 23, 2024 22:26
2uasimojo added a commit to 2uasimojo/hive that referenced this pull request Sep 27, 2024
This is a special revendor.

A prior commit (openshift#2463 / d8ebe58) included installer as a dependency in
our apis/ package for the first time. This was causing problems for
downstream consumers seemingly because the pseudo-version generated by
`go mod` represented a years-old tag with a recent commit, resulting in
excessive churn in the go sum database -- see the referenced bug report.

Now a tag has been created at that commit to make this pseudo-version
less gappy, hopefully eliminating this problem for downstreams.

So this commit revendors to that tag, which computes to the same commit,
so there is no actual change in vendoring -- only to go module metadata.

OCPBUGS-42448

HIVE-2512
2uasimojo added a commit to 2uasimojo/hive that referenced this pull request Sep 27, 2024
This is a special revendor.

A prior commit (openshift#2463 / d8ebe58) included installer as a dependency in
our apis/ package for the first time. This was causing problems for
downstream consumers seemingly because the pseudo-version generated by
`go mod` represented a years-old tag with a recent commit, resulting in
excessive churn in the go sum database -- see the referenced bug report.

Now a tag has been created at that commit to make this pseudo-version
less gappy, hopefully eliminating this problem for downstreams.

However, when I tried to revendor to that tag, go replaced it with a
pseudo-version... and then complained that we shouldn't use a
pseudo-version when there's a tag available!! (Shakes fist.)

So this commit revendors to the _next_ commit, which should mean that a
pseudo-version is the only way to reference it. The delta on that commit
is null for vendoring purposes (the changed file is not one that makes
its way into the vendor/ directory) so this is just a bump of
pseudo-versions.

OCPBUGS-42448

HIVE-2512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants