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

Internal: Set image projects via configs #1663

Merged
merged 52 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
b052962
initial commit
Subbarker Apr 2, 2024
d2ddb60
more logging
Subbarker Apr 2, 2024
45dcc4a
%v
Subbarker Apr 2, 2024
b999c4c
use platform instead of image
Subbarker Apr 3, 2024
fd1d976
Merge branch 'master' into subbarker-project-prefix
Subbarker Apr 3, 2024
f80933d
Merge branch 'master' into subbarker-project-prefix
Subbarker Apr 4, 2024
8dc21e4
remove imageProject()
Subbarker Apr 4, 2024
9c09a96
revert a good portion and move new logic behind new var: options.imag…
Subbarker Apr 4, 2024
a3e2115
remove imageProject() call
Subbarker Apr 4, 2024
07ffb99
remove extra 'nil'
Subbarker Apr 4, 2024
e5b3a28
Merge branch 'master' into subbarker-project-prefix
Subbarker Apr 4, 2024
9b1cbf2
remove ImageFamily
Subbarker Apr 5, 2024
8d8a1c5
Merge branch 'master' into subbarker-project-prefix
Subbarker Apr 5, 2024
de6708c
pass imagespec instead of platform
Subbarker Apr 5, 2024
8d475b1
call parseImageSpecin SetupVM
Subbarker Apr 5, 2024
eff145f
update soaktest SetupVM
Subbarker Apr 5, 2024
39bfca0
move parseImageSpec inside CreateInstance
Subbarker Apr 5, 2024
e0365ca
pass by reference
Subbarker Apr 5, 2024
f7d0e3f
pass by ref #2
Subbarker Apr 8, 2024
fe36fae
swap platform skip contains
Subbarker Apr 8, 2024
c3f6b1b
same as before, but updated locgic
Subbarker Apr 8, 2024
63bf469
"contains" hax
Subbarker Apr 8, 2024
2fe29e7
imports strings
Subbarker Apr 8, 2024
8903e22
update metadata.yaml files and undo contains changes
Subbarker Apr 8, 2024
22de128
bring back contains hack
Subbarker Apr 8, 2024
9377d75
remove parseimagespec from 3pappstest
Subbarker Apr 8, 2024
5a975b1
stackdriver project for sap
Subbarker Apr 8, 2024
2c1f581
print debugging arm machine type
Subbarker Apr 9, 2024
2df8960
fix print
Subbarker Apr 9, 2024
a15211a
better print debugging
Subbarker Apr 9, 2024
891b6de
undo slicecontains change
Subbarker Apr 9, 2024
44dc650
remove string import
Subbarker Apr 9, 2024
befbdd5
update sap project in kokoro config
Subbarker Apr 9, 2024
ac6b221
print debugging with stripunavailablefields
Subbarker Apr 9, 2024
16b7aee
add printf newlines
Subbarker Apr 9, 2024
d21c5ab
use HasSuffix in SliceContains
Subbarker Apr 9, 2024
e5873dd
one-line logging
Subbarker Apr 10, 2024
fdd65ab
remove print debugging
Subbarker Apr 10, 2024
f1f9863
Merge branch 'master' into subbarker-project-prefix
Subbarker Apr 10, 2024
c35be80
adressing comments
Subbarker Apr 10, 2024
fda9bb6
missing comma
Subbarker Apr 10, 2024
dc7e24e
remove old err
Subbarker Apr 10, 2024
e9ae391
formatting & comments
Subbarker Apr 10, 2024
81bf187
add constructImageSpec
Subbarker Apr 10, 2024
107bbce
remove extra "if"
Subbarker Apr 11, 2024
20adf5b
pointer math
Subbarker Apr 11, 2024
3bc535c
typo
Subbarker Apr 11, 2024
531dc84
lingering refs
Subbarker Apr 11, 2024
9bb3e00
delete oracledb to match master
Subbarker Apr 11, 2024
7d5ca3c
Merge branch 'master' into subbarker-project-prefix
Subbarker Apr 11, 2024
97e57fa
update more docs
Subbarker Apr 11, 2024
4722c08
Merge branch 'master' into subbarker-project-prefix
Subbarker Apr 11, 2024
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
2 changes: 1 addition & 1 deletion dev-docs/new-distro.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ The instructions are very similar to the instructions for `ops_agent_test`.
or
[test/third_party_apps/presubmit/bullseye_aarch64.gcl](https://github.com/GoogleCloudPlatform/ops-agent/blob/master/kokoro/config/test/third_party_apps/bullseye_aarch64.gcl),
set `platforms` to `['$DISTRO_FAMILY']` (instead of
`['debian-11']`). You should also add this section right after the
`['debian-cloud:debian-11']`). You should also add this section right after the
Subbarker marked this conversation as resolved.
Show resolved Hide resolved
`platforms =` line:

```gcl
Expand Down
2 changes: 1 addition & 1 deletion integration_test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Alternatively, you can export `PROJECT` and `TRANSFERS_BUCKET` in your
environment and simply call the target.
You can also specify the `ZONES` and `PLATFORMS` variables if you would like
to run the tests on something other than the defaults (`us-central1-b` for
ZONES and `debian-11` for `PLATFORMS`).
ZONES and `debian-cloud:debian-11` for `PLATFORMS`).

The above command will run the tests against the stable Ops Agent. To test
against a pre-built but unreleased agent, you can use add the
Expand Down
42 changes: 28 additions & 14 deletions integration_test/gce/gce_testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ AGENT_PACKAGES_IN_GCS, for details see README.md.

PROJECT=dev_project \
ZONES=us-central1-b \
PLATFORMS=debian-10,centos-8,rhel-8-2-sap-ha,sles-15,ubuntu-2004-lts,windows-2016,windows-2019 \
PLATFORMS=debian-cloud:debian-10,rocky-linux-cloud:rocky-linux-8,rhel-sap-cloud:rhel-8-2-sap-ha,suse-cloud:sles-15,ubuntu-os-cloud:ubuntu-2004-lts,windows-cloud:windows-2016,windows-cloud:windows-2019 \
go test -v ops_agent_test.go \
-test.parallel=1000 \
-tags=integration_test \
Expand Down Expand Up @@ -295,6 +295,8 @@ type VM struct {
Project string
Network string
Platform string
// The VMOptions.ImageSpec used to create the VM.
ImageSpec string
Subbarker marked this conversation as resolved.
Show resolved Hide resolved
Zone string
MachineType string
ID int64
Expand Down Expand Up @@ -1060,8 +1062,23 @@ func getVMPlatform(image string, platform string) (string, error) {
return "", errors.New("at least one of image or platform must be specified")
}
Subbarker marked this conversation as resolved.
Show resolved Hide resolved

// In cases where ImageSpec is not being used yet, construct it from known fields.
func constructImageSpec(options *VMOptions) {
if options.ImageSpec != "" || options.ImageProject == "" {
return
}
if options.Platform != "" {
options.ImageSpec = fmt.Sprintf("%s:%s", options.ImageProject, options.Platform)
} else if options.Image != "" {
options.ImageSpec = fmt.Sprintf("%s=%s", options.ImageProject, options.Image)
}
}

// parseImageSpec looks for the ImageSpec field in VMOptions and sets
// ImageProject/Image/Platform accordingly.
func parseImageSpec(options *VMOptions) (error) {
Subbarker marked this conversation as resolved.
Show resolved Hide resolved
if options.ImageSpec == "" {
constructImageSpec(options)
return nil
}

Expand All @@ -1088,9 +1105,6 @@ func parseImageSpec(options *VMOptions) (error) {
options.Image = s[1]
}

// We do this in case subsequent calls try to expand ImageSpec again.
options.ImageSpec = ""

return nil
}

Expand All @@ -1109,11 +1123,12 @@ func attemptCreateInstance(ctx context.Context, logger *log.Logger, options VMOp
return nil, err
}
vm := &VM{
Project: options.Project,
Platform: platform,
Name: options.Name,
Network: os.Getenv("NETWORK_NAME"),
Zone: options.Zone,
Project: options.Project,
Platform: platform,
ImageSpec: options.ImageSpec,
Subbarker marked this conversation as resolved.
Show resolved Hide resolved
Name: options.Name,
Network: os.Getenv("NETWORK_NAME"),
Zone: options.Zone,
}
if vm.Name == "" {
// The VM name needs to adhere to these restrictions:
Expand Down Expand Up @@ -1317,11 +1332,6 @@ func CreateInstance(origCtx context.Context, logger *log.Logger, options VMOptio
ctx, cancel := context.WithTimeout(origCtx, 3*vmInitTimeout)
defer cancel()

err := parseImageSpec(&options)
if err != nil {
return nil, err
}

shouldRetry := func(err error) bool {
// VM creation can hit quota, especially when re-running presubmits,
// or when multple people are running tests.
Expand Down Expand Up @@ -1865,6 +1875,10 @@ func SetupLogger(t *testing.T) *logging.DirectoryLogger {
type VMOptions struct {
// Optional. Can be used to pass image/image family & image project in one
// string. If set, Platform/Image/ImageProject should not be set.
Subbarker marked this conversation as resolved.
Show resolved Hide resolved
//
// Example Image Specs:
// Image Family / Project: `<project>:<family>`
// Specific Image / Project: `<project>=<image>``
ImageSpec string
// Required. Normally passed as --image-family to
// "gcloud compute images create".
Expand Down
5 changes: 1 addition & 4 deletions integration_test/metadata/integration_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"fmt"
"reflect"
"regexp"
"strings"

"cloud.google.com/go/monitoring/apiv3/v2/monitoringpb"
"github.com/go-playground/validator/v10"
Expand Down Expand Up @@ -133,9 +132,7 @@ func UnmarshalAndValidate(data []byte, i interface{}) error {

func SliceContains(slice []string, toFind string) bool {
for _, entry := range slice {
// vm.Platform (ex. `sles-15`) does not align with ImageSpec (ex. `suse-cloud:sles-15`)
// TODO, remove lingering references to "Platform" and use "Image Spec" instead.
if strings.HasSuffix(entry, toFind) {
if entry == toFind {
return true
}
}
Expand Down
2 changes: 1 addition & 1 deletion integration_test/soak_test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ parameters as desired) and run:

```
PROJECT=my_project \
DISTRO=debian-11 \
DISTRO=debian-cloud:debian-11 \
ZONES=us-central1-b \
TTL=100m \
LOG_SIZE_IN_BYTES=1000 \
Expand Down
2 changes: 1 addition & 1 deletion integration_test/soak_test/cmd/launcher/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ LOG_SIZE_IN_BYTES: How many bytes each log entry should be.

TTL: How long to keep the VM alive, expressed as "24h30m" or similar.

DISTRO: The GCE image family name to run, e.g. "debian-11".
DISTRO: The Image Spec to run the tests on, e.g. "debian-cloud:debian-11".

VM_NAME: (Optional) The name of the VM to spawn. If not supplied,
a random name will be generated by gce_testing.go.
Expand Down
14 changes: 7 additions & 7 deletions integration_test/third_party_apps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ For instructions, see the top of gce_testing.go.
This test needs the following environment variables to be defined, in addition
to the ones mentioned at the top of gce_testing.go:

PLATFORMS: a comma-separated list of distros to test, e.g. "centos-7,centos-8".
PLATFORMS: a comma-separated list of image specs to test, e.g. "suse-cloud:sles-12,ubuntu-os-cloud:ubuntu-2310-amd64".

The following variables are optional:

Expand Down Expand Up @@ -408,11 +408,11 @@ func verifyLog(actualLog *cloudlogging.Entry, expectedLog *metadata.ExpectedLog)
}

// stripUnavailableFields removes the fields that are listed as unavailable_on
// the current platform.
func stripUnavailableFields(fields []*metadata.LogFields, platform string) []*metadata.LogFields {
// the current image spec.
func stripUnavailableFields(fields []*metadata.LogFields, imageSpec string) []*metadata.LogFields {
var result []*metadata.LogFields
for _, field := range fields {
if !metadata.SliceContains(field.UnavailableOn, platform) {
if !metadata.SliceContains(field.UnavailableOn, imageSpec) {
result = append(result, field)
}
}
Expand All @@ -431,14 +431,14 @@ func runLoggingTestCases(ctx context.Context, logger *log.Logger, vm *gce.VM, lo
// underlying struct.
entry := *entry
go func() {
// Strip out the fields that are not available on this platform.
// Strip out the fields that are not available on this image spec.
// We do this here so that:
// 1. the field is never mentioned in the query we send, and
// 2. verifyLogField treats it as any other unexpected field, which
// means it will fail the test ("expected no value for field").
// This could result in annoying test failures if the app suddenly
// begins reporting a log field on a certain platform.
entry.Fields = stripUnavailableFields(entry.Fields, vm.Platform)
entry.Fields = stripUnavailableFields(entry.Fields, vm.ImageSpec)

// Construct query using remaining fields with a nonempty regex.
query := constructQuery(entry.LogName, entry.Fields)
Expand Down Expand Up @@ -499,7 +499,7 @@ func runMetricsTestCases(ctx context.Context, logger *log.Logger, vm *gce.VM, me
logger.Printf("Skipping optional or representative metric %s", metric.Type)
continue
}
if metadata.SliceContains(metric.UnavailableOn, vm.Platform) {
if metadata.SliceContains(metric.UnavailableOn, vm.ImageSpec) {
logger.Printf("Skipping metric %s due to unavailable_on", metric.Type)
continue
}
Expand Down
11 changes: 0 additions & 11 deletions kokoro/config/test/third_party_apps/release/oracledb_x86_64.gcl

This file was deleted.

2 changes: 1 addition & 1 deletion tasks.mak
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ endif
# If you would like to use a custom build you can provide a REPO_SUFFIX or
# AGENT_PACKAGES_IN_GCS. These targets function fine without either specified.
ZONES ?= us-central1-b
PLATFORMS ?= debian-11
PLATFORMS ?= debian-cloud:debian-11

integration_tests:
ZONES="${ZONES}" \
Expand Down
Loading