In case of kubernetes integration detected return manifest in standalone agent layout instead of policy #114439
Conversation
…one agent layout instead of policy
botelastic
bot
added
the
Team:Fleet
|
Pinging @elastic/fleet (Team:Fleet) |
MichaelKatsoulis
added
enhancement
| @@ -75,7 +75,7 @@ export async function getFullAgentPolicy( | |||
| id: agentPolicy.id, | |||
| outputs: { | |||
| ...outputs.reduce<FullAgentPolicy['outputs']>((acc, output) => { | |||
| acc[getOutputIdForAgentPolicy(output)] = transformOutputToFullPolicyOutput(output); | |||
| acc[getOutputIdForAgentPolicy(output)] = transformOutputToFullPolicyOutput(output, standalone); | |||
x-pack/plugins/fleet/server/services/agent_policies/full_agent_policy.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/fleet/server/services/agent_policies/full_agent_policy.ts
Outdated
Show resolved
Hide resolved
MichaelKatsoulis
added
the
release_note:skip
…d-agent-standalone-k8s
…d-agent-standalone-k8s
| import { DownloadStep, AgentPolicySelectionStep } from './steps'; | ||
| import type { BaseProps } from './types'; | ||
|
|
||
| type Props = BaseProps; | ||
|
|
||
| const RUN_INSTRUCTIONS = './elastic-agent install'; | ||
|
|
||
| export const elasticAgentPolicy = ''; |
There was a problem hiding this comment.
Yes you are right
| if (!pkg) { | ||
| return; | ||
| } | ||
| if (pkg.name === 'kubernetes') { |
There was a problem hiding this comment.
You could define a constant for that package https://github.com/elastic/kibana/blob/master/x-pack/plugins/fleet/common/constants/epm.ts/#L15
| return; | ||
| } | ||
| let found = false; | ||
| (agentPol.package_policies as PackagePolicy[]).forEach(({ package: pkg }) => { |
There was a problem hiding this comment.
I think this whole block could probably be simplified using .some (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) with something like setIsK8s((agentPol.package_policies as PackagePolicy[]).some(() => ? true : false)
| export const StandaloneInstructions = React.memo<Props>(({ agentPolicy, agentPolicies }) => { | ||
| const { getHref } = useLink(); | ||
| const core = useStartServices(); | ||
| const { notifications } = core; | ||
|
|
||
| const [selectedPolicyId, setSelectedPolicyId] = useState<string | undefined>(agentPolicy?.id); | ||
| const [fullAgentPolicy, setFullAgentPolicy] = useState<any | undefined>(); | ||
| const [isK8s, setIsK8s] = useState<string | undefined>('isLoading'); |
There was a problem hiding this comment.
using true and false as string is a little error prone what do you thing of using this types
useState<'IS_LOADING' | 'IS_KUBERNETES' | 'IS_NOT_KUBERNETES'>('IS_LOADING');
| } | ||
| }, [fullAgentPolicy, isK8s]); | ||
|
|
||
| function policyMsg() { |
There was a problem hiding this comment.
to be consistent with how we do that in other part of fleet and avoid creating a function you could do this like
const policyMsg = isK8s === 'true'
? (<Fromatred />)
: (<Fromatred />)
and later use it like
<>{policyMsg}</>
| return; | ||
| } | ||
| setYaml(fullAgentPolicy); | ||
| setRunInstructions('kubectl apply -f elastic-agent.yml'); |
There was a problem hiding this comment.
nitpick but maybe move this two cmd in a constant in the top of the file KUBERNETES_RUN_INSTRUCTIONS STANDALONE_RUN_INSTRUCTIONS
There was a problem hiding this comment.
you probably do not need a state
const runInstructions = isK8s === 'true' ? KUBERNETES_RUN_INSTRUCTIONS : STANDALONE_RUN_INSTRUCTIONS;
| if (typeof fullAgentPolicy === 'object') { | ||
| return; | ||
| } | ||
| setYaml(fullAgentPolicy); |
There was a problem hiding this comment.
You probably do not need the setYaml neither the way I would implement this useMemo return a value and compute it only if dependencies changes.
So you could do
const yaml = useMemo(() => {
return fullAgentPolicyToYaml(fullAgentPolicy);
}
There was a problem hiding this comment.
This is very tricky. The reason I did it like this is that I didn't want always this usememo to return a value for the yaml. There are unfortunately some race conditions where isk8s === true but fullAgentPolicy hasn't beed updated yet(remains an object instead of string) and also the opposite.
So the way I did it, these cases are skipped (setYaml does not run, so value of yaml remains as it was before).
If I go with the const yaml = useMemo(() => { return fullAgentPolicyToYaml(fullAgentPolicy); } approach then I always have to return a value which in those cases I shouldn't.
There was a problem hiding this comment.
In this case you should probably use useEffect instead of useMemo
| const body = fullAgentConfigMap; | ||
| const headers: ResponseHeaders = { | ||
| 'content-type': 'text/x-yaml', | ||
| 'content-disposition': `attachment; filename="elastic-agent.yml"`, |
There was a problem hiding this comment.
Do we need to have generate a different filename for the manifest?
There was a problem hiding this comment.
we could call it elastic-agent-standalone-kubernetes.yml like in https://github.com/elastic/beats/blob/master/deploy/kubernetes/elastic-agent-standalone-kubernetes.yaml
| }; | ||
|
|
||
| const configMapYaml = fullAgentConfigMapToYaml(fullAgentConfigMap, safeDump); | ||
| const updateMapHosts = configMapYaml.replace('http://localhost:9200', '{ES_HOST}'); |
There was a problem hiding this comment.
It will not be localhost:9200 in an other environment than your local dev one. We should probably allow to pass this as an option to getFullAgentPolicy or we can rely on the user setting the correct value in the Fleet settings and remove that line.
There was a problem hiding this comment.
Yes I thought of that also. I decided to pass it as an option to getFullAgentPolicy. The problem is that the outputs is an array and the hosts of each array is also an array. Not sure how multiple outputs and multiple hosts per output in a policy can be configured in the UI though. I can just change the first ones of the array.
There was a problem hiding this comment.
@MichaelKatsoulis Normally it's the responsibility of the user to configurate this hosts correctly in Fleet, so I think we should not replace it here.
|
@nchaulet Thanks for your review. I updated the PR after your comments |
…d-agent-standalone-k8s
x-pack/plugins/fleet/server/services/agent_policies/full_agent_policy.ts
Outdated
Show resolved
Hide resolved
|
@elasticmachine merge upstream |
…d-agent-standalone-k8s
…MichaelKatsoulis/kibana into update-fleetui-add-agent-standalone-k8s
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/search/session·ts.apis search search session touched time updates when you poll on an searchStandard OutStack TraceMetrics [docs]Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
kibanamachine
added
the
backport missing
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
@MichaelKatsoulis Look like this PR has not been backported 7.x and 7.16 |
nchaulet
added
v8.0.0
auto-backport
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…one agent layout instead of policy (elastic#114439) * In case of kubernetes integartion detected return manifest in standalone agent layout instead of policy
…one agent layout instead of policy (#114439) (#115953) * In case of kubernetes integartion detected return manifest in standalone agent layout instead of policy Co-authored-by: Michael Katsoulis <[email protected]>
mlunadia
changed the title
…
Summary
After discussions in #110408 , this PR
In case no k8s integration detected in policy:
In case k8s integration detected:
Checklist