feat: update hub-agent to use custom cluster affinity plugin in scheduler when property checker is enabled #1221
Conversation
britaniar
force-pushed
the
addPropertyChecker
branch
from
da06f2a to
f3fd2a3
Compare
britaniar
force-pushed
the
addPropertyChecker
branch
from
3ca7be3 to
a302d9f
Compare
britaniar
force-pushed
the
addPropertyChecker
branch
2 times, most recently
from
6f94158 to
0dcc8a8
Compare
britaniar
force-pushed
the
addPropertyChecker
branch
from
ed3fb63 to
cdb5e9f
Compare
zhiying-lin
left a comment
zhiying-lin
left a comment
There was a problem hiding this comment.
could you please update the PR descriptions and title to reflect the latest changes?
britaniar
changed the title
britaniar
force-pushed
the
addPropertyChecker
branch
from
cdb5e9f to
85a9ea8
Compare
britaniar
force-pushed
the
addPropertyChecker
branch
3 times, most recently
from
992ddbf to
9438540
Compare
Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
britaniar
force-pushed
the
addPropertyChecker
branch
from
9438540 to
0e92d1a
Compare
| } | ||
|
|
||
| // AddFlags adds flags for Azure Property Checker options to the given FlagSet. | ||
| func (o AzurePropertyCheckerOptions) AddFlags(flags *flag.FlagSet) { |
There was a problem hiding this comment.
I'm not sure if we want to make this a pointer. cc @zhiying-lin like RatelimiterOpts: https://github.com/kubefleet-dev/kubefleet/blob/main/cmd/hubagent/options/ratelimit.go#L43
There was a problem hiding this comment.
my bad, we have to use the pointer in this case, otherwise, when we check if opts.AzurePropertyCheckerOpts.IsEnabled {, it's using original value, as the flag is using the copy of the option.
flags.BoolVar(&o.IsEnabled, "azure-property-checker-enabled", o.IsEnabled, "Enable Azure property checker for validating Azure-specific cluster properties.")
There was a problem hiding this comment.
btw, here's the doc about receiver type, https://google.github.io/styleguide/go/decisions#receiver-type
| } | ||
|
|
||
| // NewAzurePropertyCheckerOptions creates a new AzurePropertyCheckerOptions with default values. | ||
| func NewAzurePropertyCheckerOptions() AzurePropertyCheckerOptions { |
There was a problem hiding this comment.
ditto, do we want to return a pointer instead?
There was a problem hiding this comment.
i feel value is better here since the caller needs the value instead of the pointer.
4 participants
Description of your changes
I have:
Hub agent reads in a Boolean to enable property checker functionalities and passes the compute service endpoint base url.
Use newly added options to create scheduler with a custom cluster affinity plugin to use property checker.
Run
make reviewableto ensure this PR is ready for review.How has this code been tested
Logs from hub agent used for testing:
I1106 19:15:29.423534 1 workload/setup.go:389] Setting up scheduler I1106 19:15:29.423560 1 workload/setup.go:392] Azure property checker is enabled for cluster property validation I1106 19:15:29.423626 1 workload/setup.go:408] Starting the schedulerSpecial notes for your reviewer
There are some copied files from previous PR so that the setup could work.